Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to google-auth from oauth2client which is now deprecated #89

Open
shcheklein opened this issue Feb 21, 2021 · 8 comments
Open
Labels
enhancement New feature or request help wanted Extra attention is needed priority-p1

Comments

@shcheklein
Copy link
Member

More info here https://google-auth.readthedocs.io/en/latest/oauth2client-deprecation.html

@shcheklein shcheklein added enhancement New feature or request help wanted Extra attention is needed labels Feb 21, 2021
@junpeng-jp
Copy link

hey @shcheklein, I would like to help migrate to google-auth & google-auth-oauthlib

What are the plans for using oauth2client.Storage in the future for persisting credentials in a thread-safe manner? Storage is not implemented in google-auth (see googleapis/google-auth-library-python#33)

I'm not familiar with what multi-threading but is there a way to share credentials across threads and handle the token refresh, reading, writing, etc from the shared credentials object?

@shcheklein
Copy link
Member Author

@junpeng-jp there is more than MT to this. PyDrive2 is relying on this implementation https://github.com/googleapis/oauth2client/blob/master/oauth2client/file.py to save / read credentials from a file (that is specified in the settings.yaml).

My take on this that initially we can get the whole Storage (abstract class) and the file implementation into the PyDrive2 itself. They are small. Add tests to cover the file based impl.

Next step would be to is check how this object is implemented now https://github.com/googleapis/oauth2client/blob/master/oauth2client/client.py#L441 .

It interacts with the storage instance to keep updating credentials in it. It might turn out we would need to write a wrapper for the new credentials abstraction. Or it has some param (store) to pass.

@NickCrews
Copy link

Driveby-ing here, but perhaps you could look at https://github.com/burnash/gspread and see what they do. These two projects seem to be very related, perhaps you could share the load and come up with something that both libraries could use.

@junpeng-jp
Copy link

junpeng-jp commented May 7, 2022

@shcheklein Took a look at the Storage class and I think I can just update the refresh method for the google Credential object to have the above locked file read/write behaviour. This should allow AuthorizedSession / AuthorizedHttp to handle the storing of tokens using a re-implemented Storage object within the self.credential.before_request calls.

@shcheklein
Copy link
Member Author

@junpeng-jp could you please put some links so that we are on the same page on what specific calls / classes we are talking about?

@junpeng-jp
Copy link

junpeng-jp commented May 8, 2022

@shcheklein Scratch what I said previously. I found that the old google oauth2client's Storage is used for post-refresh token read / write in Client. To replicate this, I think we need 2 things:

  • a copy of the Storage object with the same file locking capabilities
  • a custom Credential object that has self.store & modify the refresh method (see here) to save the new access & refresh tokens every time a refresh occurs

The Credential's refresh method is called in in 2 places within the self.request method of AuthorizedSession and AuthorizedHttp:

  • first refresh can happen via the self.credential.before_request call
  • subsequent refreshes can happen later in the method with up to 2 retries

@shcheklein
Copy link
Member Author

@junpeng-jp yep, it makes sense, thanks.

@adrien-n
Copy link

Hi, python-auth2client is really very deprecated. Dead might actually be a better description.

I'm looking at this as part of Ubuntu maintenance: the new pyopenssl version dropped an API which python-auth2client uses and a fix is not completely trivial and I'm not willing to do it since there seems to be space to do it quite poorly. There were also issues a few months ago with new python versions (something to do with asserts IIRC).

The patches sent from Ubuntu to Debian for the assert issue have not been merged and I'm not sure the package is still actually usable in Debian. I think it should be removed but obviously this can't really be done if other packages depend upon it. In any case, it seems that dependency is unfortunately less and less usable, no matter the distribution.

PS: since I'm not familiar with the project, I don't know the impact of removing/disabling the corresponding bits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed priority-p1
Projects
None yet
Development

No branches or pull requests

4 participants