Migrating to Google Auth Library #221

junpeng-jp · 2022-08-06T13:56:10Z

Summary of changes

#89

Removed all dependencies on oauth2client and transitioned to the use of google-auth

#89

Added a thread-safe backend adaptor for saving of authorized credentials to File / Dictionary Backend
Only authorized user credentials can be saved to backend
Service credential are not saved because of private key duplication risks

#173

Deprecate command line auth by raising a DeprecationWarning Exception

Removing manual refresh & authorization header injection code as this is now handle by the Google Auth library

Removal of CheckAuth & CheckServiceAuth decorators
Deprecation of self.Refresh() & self.Authorize() methods
Update self.Get_Http_Object() method to use the AuthorizedHttp object from google's HTTP migration helper library

junpeng-jp · 2022-08-09T15:54:32Z

@shcheklein I've raise a new PR above. Please do take a look when you have time

shcheklein · 2022-08-10T00:15:38Z

@junpeng-jp hey, sure. Please don't hesitate to Request a review via a link at the top. Otherwise it was not clear if it's still in progress or not. Thanks again for the effort. I'll try to find time to review it this week.

junpeng-jp · 2022-08-14T12:22:11Z

@shcheklein Not sure why but I don't see a button to request a reviewer for this PR. Are you able to help me here?

Screenshot of what I see below

shcheklein · 2022-08-14T16:27:39Z

@junpeng-jp ah, no worries. If it doesn't work- that's fine. Just ping me in the comment.

Sorry for the delay, this PR needs more attention and I need to find time to check it out.

shcheklein · 2022-08-14T23:38:19Z

pydrive2/auth.py

@@ -1,28 +1,31 @@
 import json
 import webbrowser
 import httplib2
-import oauth2client.clientsecrets as clientsecrets
+import json


we have import json already ^^

shcheklein · 2022-08-14T23:39:39Z

pydrive2/auth.py

-    """Decorator to check if the auth is valid and loads auth if not."""
+    """
+    Decorator to check the self.auth & self.http object in a decorated API call.
+    Loads a new GoogleAuth or Http object if not the needed.


if not the needed - needs to be fixed

shcheklein · 2022-08-14T23:46:11Z

pydrive2/auth.py

+        self._credentials = None
+
+    # Lazy loading, read-only properties
+    # these look like functions but are actually getters for the read only properties


I don't think we need this explanation, it's quite common pattern

almost all of these properties should be private? which ones do we want to expose?

shcheklein · 2022-08-14T23:54:21Z

pydrive2/storage.py

+
+
+class CredentialBackend(object):
+    """Adapter that provides a consistent interface to read and write credential files"""


write credential files -> write credentials

shcheklein · 2022-08-15T00:01:23Z

pydrive2/auth.py


        try:
-            self.credentials = self._default_storage.get()
+            auth_info = self.default_storage.read_credentials(credentials_file)


I think storage should be per file name. Otherwise we have to keep providing credentials_file everywhere + we use the same lock for all credential files potentially

shcheklein · 2022-08-15T00:04:55Z

pydrive2/auth.py

+                auth_info, scopes=auth_info["scopes"]
+            )
+        except ValueError:
+            warn(


again, could you remind me please about the use case? can we drop this?

shcheklein · 2022-08-15T00:07:58Z

pydrive2/auth.py

            if self._default_storage is None:
                raise InvalidConfigError(
                    "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)
+            self._default_storage = FileBackend()


information is lost at this point that this storage is from the specified credentials_file. I'm not 100% understand the use case why would someone do auth.LoadCredentialsFile("some/custom/path"), but it looks like we are changing the behavior here ...

okay, before it was important since the credentials object was triggering save on refresh, not sure again yet if this is important here ...

shcheklein · 2022-08-15T03:56:37Z

pydrive2/auth.py

-        self.service = build(
-            "drive", "v2", http=self.http, cache_discovery=False
+        raise DeprecationWarning(
+            "Manual authorization of HTTP will be deprecated as the"


hmm, but we do have AuthorizedHttp(...) call which is similar to the self.credentials.authorize(self.http). In this case again, I'm not sure why it (the public Authorize call) was needed in the first place. Probably for some custom auth flow. Would be great to get more information on this.

Authorize is being used in the Auth above that won't work anymore. And it's needed for custom flow logic like this https://docs.iterative.ai/PyDrive2/oauth/#building-your-own-authentication-flow .

shcheklein · 2022-08-15T05:05:17Z

pydrive2/auth.py

-        except AccessTokenRefreshError as error:
-            raise RefreshError("Access token refresh failed: %s" % error)
+        raise DeprecationWarning(
+            "Manual refresh will be deprecated as the"


is deprecated ...

shcheklein · 2022-08-16T01:29:23Z

pydrive2/auth.py


+    @property
+    def credentials(self):
+        if not self._credentials:


why do we need / have to support this as a lazy / self-initializing logic for credentials?

btw, have you seen the:

client_config = ApiAttribute("client_config") flow = ApiAttribute("flow") credentials = ApiAttribute("credentials") http = ApiAttribute("http") service = ApiAttribute("service") auth_method = ApiAttribute("auth_method")

how those coexist with these properties?

lappazos · 2022-10-23T07:35:06Z

@shcheklein @junpeng-jp any update on this PR? I really need the automatic auth using GCP service account credentials.

shcheklein · 2022-10-23T17:08:05Z

@lappazos please give it a try https://docs.iterative.ai/PyDrive2/oauth/#authentication-with-a-service-account . You don't need to wait for this PR to be merged.

lappazos · 2022-11-02T10:47:26Z

@shcheklein so i did - i couldn't find where this function is implemented.
i tried to implement it by myself (copy pate), but the GCP environment couldn't find the "client_json_file_path": "service-secrets.json",

shcheklein · 2022-11-02T19:21:13Z

@lappazos please, let's move this to a separate issue/discussion. This is not related I think to this PR.

lappazos · 2023-03-15T05:30:17Z

@junpeng-jp @shcheklein is this PR stuck?

shcheklein · 2023-04-09T19:42:39Z

Closing as stale

el-abcd · 2023-11-05T22:43:01Z

Just noting that it would be nice if this PR can be "brought back to life/completed" to enable features like "service account impersonation", etc.

Background:
I was working on a small hobby project (and attempting to learn some of the "best practices" for service accounts/authentication, etc.).

IIUC, it seems like using "service account impersonation" is preferred to downloading a "service account key", as the key is long-lived, requires manual rotation, etc. Whereas "impersonation" allows you to just fetch a temporary token for the service account (using your existing user credentials)
service account key

While I'm not 100% certain it is required, I think that using the updated google oauth libraries are probably the best way to enable these features.
And I did have success using the https://github.com/junpeng-jp/PyDrive2 fork for this...

I was able to get it running locally (using mostly default settings for PyDrive2, and creating "impersonated application-default credentials" with:

gcloud auth application-default login --impersonate-service-account SERVICE_ACCT_EMAIL

To make the "application default credentials" be the "impersonated service account" ones.

I was ALSO able to get a test version running on a google cloud run container (where a google managed "service account" is used to run the service, and I was able to get that service account to "impersonate" an existing service account (which already has access to certain drive folders) and get things working that way also. It did require a bit of work (monkey patching the GoogleAuth.ServiceAuth() method to set self._credential to be the impersonated one). I think that would be a "fairly small update" to allow impersonated service accounts cleanly (if this PR is revived).

At any rate, just noting that this PR does appear to be pretty useful, and hoping that this can be brought back to life / completed (as it appears to be pretty far along / functional).

Eric

junpeng-jp added 3 commits August 6, 2022 21:25

Migrate to Google Auth

2789fda

Raise deprecation errors for Refresh & Authorize methods

d97357f

Remove default service autth for future iterations

9e51121

junpeng-jp had a problem deploying to external August 6, 2022 13:56 Failure

junpeng-jp mentioned this pull request Aug 6, 2022

Migrating to Google Auth Library #180

Closed

junpeng-jp marked this pull request as ready for review August 7, 2022 14:31

junpeng-jp marked this pull request as draft August 14, 2022 11:56

junpeng-jp marked this pull request as ready for review August 14, 2022 11:56

Merge branch 'iterative:main' into dev

f3b4b68

junpeng-jp had a problem deploying to external August 14, 2022 12:09 Failure

shcheklein self-requested a review August 14, 2022 16:26

shcheklein reviewed Aug 14, 2022

View reviewed changes

shcheklein reviewed Aug 15, 2022

View reviewed changes

shcheklein reviewed Aug 16, 2022

View reviewed changes

junpeng-jp added 2 commits August 28, 2022 12:39

Updated documentation

55ec88c

Merge branch 'dev' of https://github.com/junpeng-jp/PyDrive2 into dev

57bcee5

junpeng-jp had a problem deploying to external August 28, 2022 04:41 Failure

Merge branch 'main' of https://github.com/iterative/PyDrive2 into dev

48b9266

junpeng-jp had a problem deploying to external August 28, 2022 04:42 Failure

Update more documentation

c4b0001

junpeng-jp had a problem deploying to external August 28, 2022 04:47 Failure

junpeng-jp added 3 commits September 3, 2022 13:34

Removed ApiAttribute descriptors in Google Auth

15fe922

Changed FileBackend to read/write to single file only

ebf0ccc

Removed the use of Authorize method in self.Auth

da60662

junpeng-jp had a problem deploying to external September 3, 2022 05:36 Failure

Add test case for custom google auth

a7e20d0

junpeng-jp had a problem deploying to external September 3, 2022 06:51 Failure

junpeng-jp added 2 commits September 3, 2022 15:01

Fixed the behaviour of GetAuthUrl & Authenticate

42e327c

Cleanup Credentials Backend code

c8bd17f

junpeng-jp had a problem deploying to external September 3, 2022 07:10 Failure

Merge branch 'main' of https://github.com/iterative/PyDrive2 into dev

d476f43

junpeng-jp had a problem deploying to external September 3, 2022 07:15 Failure

atwater-tempus mentioned this pull request Sep 12, 2022

Confirm OAuth OOB and Implement Temporary Workaround #231

Open

lappazos mentioned this pull request Nov 6, 2022

authentication-with-a default service-account isn't working #240

Open

shcheklein closed this Apr 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrating to Google Auth Library #221

Migrating to Google Auth Library #221

junpeng-jp commented Aug 6, 2022

junpeng-jp commented Aug 9, 2022

shcheklein commented Aug 10, 2022

junpeng-jp commented Aug 14, 2022

shcheklein commented Aug 14, 2022

shcheklein Aug 14, 2022

shcheklein Aug 14, 2022

shcheklein Aug 14, 2022

shcheklein Aug 14, 2022

shcheklein Aug 14, 2022

shcheklein Aug 15, 2022

shcheklein Aug 15, 2022

shcheklein Aug 15, 2022

shcheklein Aug 15, 2022

shcheklein Aug 15, 2022

shcheklein Aug 15, 2022 •

edited

Loading

shcheklein Aug 15, 2022

shcheklein Aug 16, 2022

lappazos commented Oct 23, 2022

shcheklein commented Oct 23, 2022

lappazos commented Nov 2, 2022

shcheklein commented Nov 2, 2022

lappazos commented Mar 15, 2023

shcheklein commented Apr 9, 2023

el-abcd commented Nov 5, 2023 •

edited

Loading



		class CredentialBackend(object):
		"""Adapter that provides a consistent interface to read and write credential files"""

Migrating to Google Auth Library #221

Migrating to Google Auth Library #221

Conversation

junpeng-jp commented Aug 6, 2022

Summary of changes

junpeng-jp commented Aug 9, 2022

shcheklein commented Aug 10, 2022

junpeng-jp commented Aug 14, 2022

shcheklein commented Aug 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shcheklein Aug 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lappazos commented Oct 23, 2022

shcheklein commented Oct 23, 2022

lappazos commented Nov 2, 2022

shcheklein commented Nov 2, 2022

lappazos commented Mar 15, 2023

shcheklein commented Apr 9, 2023

el-abcd commented Nov 5, 2023 • edited Loading

shcheklein Aug 15, 2022 •

edited

Loading

el-abcd commented Nov 5, 2023 •

edited

Loading