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

Google installed app loopback redirect OAuth flow #369

Closed
wants to merge 4 commits into from

Conversation

bclarkx2
Copy link

@bclarkx2 bclarkx2 commented Aug 28, 2022

As per Making Google OAuth interactions safer by using more secure OAuth flows, Google is deprecating the Out-Of-Band OAuth flow for installed desktop apps that grive2 currently uses on October 3, 2022. I'm actually not 100% if this includes requests for new access tokens given an existing, valid refresh token.

Following the Out-Of-Band (OOB) flow Migration Guide, this PR attempts to migrate the app to use the Loopback IP address flow. It does this by spinning up a small HTTP server on localhost to accept the client-side redirect from the Google user consent flow.

Key CR considerations:

  • I added a dependency on cpprestsdk to simplify the process of creating an HTTP listener on localhost. Not sure if that's kosher, so lmk if there's a better way to do this! In particular, I don't think this particular dependency is currently packaged on Alpine Linux which I saw the included Dockerfile using.
  • I really don't know much about either C++ or CMake, but AFAICT I did need to explicitly link OpenSSL. I think that's OK since it's a runtime dynamic link (I think lol), but again lmk if that's going to be a problem.
  • I didn't update the version from 0.5.2-dev since I wasn't sure if you'd prefer to bundle updates into releases, but I am happy to increment this in this PR if preferred!

@bclarkx2 bclarkx2 marked this pull request as ready for review August 28, 2022 20:35
@nettlep
Copy link

nettlep commented Sep 1, 2022

@bclarkx2 - Thanks for doing this! I have tried this branch and although I was able to get it working, I did run into issues.

The first issue is that when running grive -a it reports the server is listening on http://localhost:9898 but it would not accept my connection. A call to netstat found that it was only listening on ipv6. I wasn't able to find a way to connect to it through the browser (including modifying the URL to include the ipv6 address in square brackets) in order to finish the auth flow. I did a bit of googling for this issue with cpprest and nothing jumped out at me (not a lot of time available to diagnose.) Instead, I tried docker and was able to get a valid .grive file that way (it prompted to paste the credentials rather than using a listening server.)

The second issue I ran into was that the resulting .grive file worked in docker, but not when I copied it out of the container and over to my local box where I needed it (also verified running the updated code.) When used on my local box, running 'grive' reported that I should first run grive -a (even though the .grive file was right there.) I did some debugging and found that it was looking for redirect_uri inside the .grive file, but it did not exist. So I added it manually.

From this point, all worked well.

This may very well be something I'm doing wrong or some odd configuration on my box causing issues, but I wanted to share just in case I've found some odd edge-case.

Thanks again for the updates!

@tom-ch1
Copy link

tom-ch1 commented Sep 2, 2022

Thanks Brian,
your patch worked for me and will presumably solve the oob deprecation problem.
I would only suggest 2 edits for the documentation in README.md:

  • Chapter "Usage" still suggests using grive -a for the first time. Shouldn't that be grive -a --id <client_id> --secret <client_secret>?
  • Chapter "Installation" : should mention the cpprestsdk and how to satisfy it.

What is to correct way for me to add those edits? In a fork of mine? How can I "attach" it to your pull request (That edit would only make sense if your PR is accepted).

Thanks for the work!
Cheers, Tom

@bclarkx2
Copy link
Author

@nettlep thanks!

The first issue is that when running grive -a it reports the server is listening on http://localhost:9898 but it would not accept my connection. A call to netstat found that it was only listening on ipv6. I wasn't able to find a way to connect to it through the browser (including modifying the URL to include the ipv6 address in square brackets) in order to finish the auth flow. I did a bit of googling for this issue with cpprest and nothing jumped out at me (not a lot of time available to diagnose.)

Very interesting! At least on my machine, ss reports that it appears to be listening for TCP connections at the IPv4 address of 127.0.0.1:9898:

$ ss -ap '( sport = :9898 )'
Netid                State                 Recv-Q                Send-Q                               Local Address:Port                               Peer Address:Port               Process
tcp                  LISTEN                0                     128                                      127.0.0.1:9898                                    0.0.0.0:*                   users:(("grive",pid=95108,fd=6))

I wonder, does it work for you if you provide --redirect-uri=127.0.0.1:9898 as a command line flag? Perhaps you have an OS setting that resolves localhost automatically to the IPv6 loopback address 🤔

Instead, I tried docker and was able to get a valid .grive file that way (it prompted to paste the credentials rather than using a listening server.)

This project's Dockerfile actually appears to always build from the vitalif/grive2 @ master branch: https://github.com/vitalif/grive2/blob/master/Dockerfile#L6. That should explain why it was prompting you to paste credentials (i.e., use the deprecated OOB flow) rather than the loopback address flow.

The second issue I ran into was that the resulting .grive file worked in docker, but not when I copied it out of the container and over to my local box where I needed it (also verified running the updated code.) When used on my local box, running 'grive' reported that I should first run grive -a (even though the .grive file was right there.) I did some debugging and found that it was looking for redirect_uri inside the .grive file, but it did not exist. So I added it manually.

This is a solid point! If you have a valid .grive file with a valid refresh_token, I don't see any reason why you should be required to supply a redirect URI just to trigger a sync. In ec50a56, I retooled the way it sets up the redirect-uri config to allow a sync to run unimpaired with a refresh_token. When you use the -a option, it will then use the default redirect-uri (or whatever value you provided via the --redirect-uri command line option) and store that value to the config file.

I hope that will resolve the issue!

@bclarkx2
Copy link
Author

@tom-ch1 great, glad to hear it worked!

Thanks Brian, your patch worked for me and will presumably solve the oob deprecation problem. I would only suggest 2 edits for the documentation in README.md:

  • Chapter "Usage" still suggests using grive -a for the first time. Shouldn't that be grive -a --id <client_id> --secret <client_secret>?

I actually think that grive -a is probably still the right default instructions to give. The "default" grive2 Google OAuth app still exists and actually seems to work for me today. I know there were issues in the past about quotas and app approval (see Different OAuth2 client to workaround over quota and google approval issues for more details) which is why they started recommending using your own OAuth client credentials. But, it seems to me like that hasn't replaced grive -a as the default way to use the tool, since I think the ideal path would be that the quota and approval issues would be solved one day and most people could go back to using the official grive2 OAuth app.

  • Chapter "Installation" : should mention the cpprestsdk and how to satisfy it.

I added libcpprest-dev to the list of required libraries here, as well as to the instructions for setup on Debian, Fedora, and FreeBSD -- did you have any other sort of documentation in mind? I did go ahead and make that library name a hyperlink, in case that's the sort of attribution you were thinking of.

Thanks for the feedback!

@nettlep
Copy link

nettlep commented Sep 12, 2022

@bclarkx2 -

For the ipv6 issue - you nailed it. It was my hosts file. I'm pretty sure I'm still running the default hosts file from Ubuntu 20.04.4 LTS). This is the hosts file verbatim (the problem is the line under the comment):

127.0.0.1   localhost.localdomain   localhost
::1     localhost6.localdomain6 localhost6

# The following lines are desirable for IPv6 capable hosts
::1     localhost ip6-localhost ip6-loopback
fe00::0 ip6-localnet
ff02::1 ip6-allnodes
ff02::2 ip6-allrouters
ff02::3 ip6-allhosts

I decided to try again, so I commented out the problematic line and found another issue. This time, it properly listened on 127.0.0.1:9898. Remember, though, this server is headless, so I'm left trying to connect from my laptop, but it will only accept connections from localhost. Using lynx solved the problem quickly enough. I tried using a redirect_uri of 0.0.0.0:9898 but Google wouldn't accept it. Might want to add a note to the documentation about this.

I actually did try the redirect_uri but only ever used localhost with it. I outsmarted myself, apparently.

Thanks again for following up. Hopefully this gets accepted into the main repo (looking at you @vitalif !)

@abdullah-kasim
Copy link

I was getting error 500, and escapinghttp://localhost:9898 in the query string might have fixed it. I'm not sure what fixed my issue, as I also brought my google oauth app from production back to testing.

@abartov
Copy link

abartov commented Oct 17, 2022

Thank you for this patch; it worked for me!

@hubyhuby
Copy link

hubyhuby commented Nov 7, 2022

Do you have any timeline to merge this patch ?
Grive2 stopped working :
image

Moved to ocalm for now it works with new google drive requrirements : https://github.com/astrada/google-drive-ocamlfuse/

@vitalif
Copy link
Owner

vitalif commented Nov 9, 2022

Lol. As always I first did something and then looked at PRs :D

@vitalif
Copy link
Owner

vitalif commented Nov 9, 2022

Sorry :D

@vitalif vitalif closed this Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants