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

gdrive: use LocalWebserverAuth instead of CommandLineAuth #52

Merged
merged 1 commit into from
Jun 15, 2022

Conversation

dtrifiro
Copy link
Contributor

pydrive2.auth.CommandLineAuth uses out of band authentication (urn:ietf:wg:oauth:2.0:oob) which has been deprecated, breaking authentication for gdrive with oauth.

Switching to LocalWebserverAuth solves the issue (thanks @shcheklein)

fixed iterative/dvc#7867

@skshetry
Copy link
Member

For the record, 3.10 is failing due to a different issue. Something went wrong with .nox caching. :(

@shcheklein
Copy link
Member

We need to see if we need to update the setting with the built-in Python app. @dtrifiro could you please test w/o feeding any custom secrets, etc? It should be working also.

Also, let's check if we need to update docs (application setup in the Google Cloud is a bit different - you need to allow it to redirect to localhost something where this LocalWebserverAuth is listening).

Also, need to check what happens if both 8080 and 9090 (I think it tries those two to run the localwebserver) are taken already. Ideally we should probably create a config option to setup this. (this not critical I think).

@dtrifiro
Copy link
Contributor Author

dtrifiro commented Jun 13, 2022

We need to see if we need to update the setting with the built-in Python app. @dtrifiro could you please test w/o feeding any custom secrets, etc? It should be working also.
Also, let's check if we need to update docs (application setup in the Google Cloud is a bit different - you need to allow it to redirect to localhost something where this LocalWebserverAuth is listening).

It works with no custom secrets, as client id/secret are retrieved automatically: the part about setting client secrets in the dvc config can be removed from the documentation. edit: this is not correct: still need client secret/id for custom OAuth apps

Also, need to check what happens if both 8080 and 9090 (I think it tries those two to run the localwebserver) are taken already. Ideally we should probably create a config option to setup this. (this not critical I think).

Returns an authentication error with clear information about what's going on:

Failed to start a local web server. Please check your firewall
settings and locally running programs that may be blocking or
using configured ports. Default ports are 8080 and 8090.

Note that these two ports are hardcoded in LocalWebserverAuth


Thinking about it some more though, using `LocalWebserverAuth` instead of `CommandLineAuth` is not ideal when running `dvc` on a remote server: in that case even if the user opens the link, there would be no listening local web server, so the authentication would be once again broken.

We could suggest a (janky) workaround in the docs using ssh -L 8080:localhost:8080 <host> to forward requests to the localhost:8080 on the client to the server on which dvc is running. This would only work in the scenarios in which the user actually has ssh access to the server and AllowTcpForwarding (for OpenSSH) is not disabled on the server side.

If we do not want to go ahead with the LocalWebserverAuth setup, we could wait for https://github.com/iterative/PyDrive2/pull/180/files to be merged, which does solve the issue.

@shcheklein
Copy link
Member

thanks @dtrifiro !

the part about setting client secrets in the dvc config can be removed from the documentation

which one? could you clarify please?

Returns an authentication error with clear information about what's going on:

👍

We could suggest a (janky) workaround in the docs using ssh -L 8080:localhost:8080

good as a workaround.

also they can run it once locally and copy the credentials file to the remote machine

or, we they are creating their own custom APP they could setup it in a some different way (provide an IP of the machine to redirect to) - we would need to expose some extra settings though for the remote

If we do not want to go ahead with the LocalWebserverAuth setup

hmm, I'm not sure it solves it, though ... the whole CommandLineAuth gets deprecated, right? it won't work anymore at all. Even in that PR it is disabled (redirected to the LocalWebserverAuth as far as I remember).

@dtrifiro
Copy link
Contributor Author

dtrifiro commented Jun 14, 2022

which one? could you clarify please?

I meant the client secret/id. I will open a PR on the docs asap, edit: not required, setup guide will not have to be changed.

hmm, I'm not sure it solves it, though ... the whole CommandLineAuth gets deprecated, right? it won't work anymore at all. Even in that PR it is disabled (redirected to the LocalWebserverAuth as far as I remember).

Oh, you're right:

"The command line auth has been deprecated. "
"The recommended alternative is to use local webserver auth with a loopback address."

I guess this will be fine then.

dtrifiro added a commit to dtrifiro/dvc.org that referenced this pull request Jun 14, 2022
dvc used Pydrive2's `CommandLineAuth` mechanism, which used the
now-deprecated OAuth2 out-of-band authorization. This was replaced with
`LocalWebserver` authentication, which no longer requires manual setup
of client id/secret.

iterative/dvc-objects#52
@dtrifiro
Copy link
Contributor Author

dtrifiro commented Jun 15, 2022

@shcheklein I can confirm that the OAuth flow works with both the default DVC app and a custom application (custom client secret/id). You can disregard my previous comments about client id/secret and docs update: setup steps remain unchanged.

@shcheklein
Copy link
Member

We still need to update docs? For a custom APP - does "Desktop App" type still works and do users need to allow redirects to localhost:8080 or localhost:9090 - or it's enabled by default?

@dtrifiro dtrifiro requested a review from skshetry June 15, 2022 21:38
@dtrifiro dtrifiro merged commit b889cd7 into iterative:main Jun 15, 2022
@dtrifiro
Copy link
Contributor Author

We still need to update docs? For a custom APP - does "Desktop App" type still works and do users need to allow redirects to localhost:8080 or localhost:9090 - or it's enabled by default?

@shcheklein We don't need to update the docs as far as I can tell: when creating a "Desktop App" it just works with no extra configuration. From my testing (for an "internal" app) just following the guide is enough:

  1. create a new project
  2. enable the drive API
  3. create a new app on the OAuth Consent Screen section
  4. create new oauth credentials (application type: "Desktop app") on the Credentials section

As far as I can tell, using an "external" app should not change this. Here are the docs.

If by "allow redirects" you mean setting up authorized URLs in the OAuth Consent Screen section of "APIs and Services", the answer is no: it just works and it's not even possible to manually add localhost:

image

Also see https://developers.google.com/identity/protocols/oauth2/native-app#step-2:-send-a-request-to-googles-oauth-2.0-server

@shcheklein
Copy link
Member

I see, I meant this screen (that applies to Web App custom app type_:

Screen Shot 2022-06-15 at 7 18 13 PM

But if redirects work fine for Desktop App then we don't need anything at all (potentially later, we can add some config to pass port / host name I think). Thanks @dtrifiro !

@dtrifiro dtrifiro deleted the fix-gdrive-oauth branch June 16, 2022 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

gdrive: oauth authentication broken
3 participants