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

feat(providers): Support interactive oauth setup for gdrive. #3288

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xkxx
Copy link
Contributor

@xkxx xkxx commented Sep 11, 2023

Fixes #3047, #2656

Hi @jkowalski,

Would you take an early look at this PR:

  • The changes in repo/ and cli/ are small and ready more or less.
  • The changes in internal/gdriveauth/ is more POC-quality (see below).

I would like your opinion on how the ephemeral http server is structured. You mentioned that you may want to add more use cases in the future, so maybe a consistent architecture would help? I also don't work with go serving on my day job, so feedback is much appreciated here.

I also marked the existing service account-based auth as deprecated in the CLI, although it should work. It's my opinion that we should mark it obsolete and remove it soon, since the gotchas with the quota is great and confusing.

The overall flow of the code:

  • In repository create, we launch a http server that serves 2 ajax endpoints and a html file.
  • The user is asked to authenticate and then authorize kopia. If it goes well, we receive an auth code.
  • The auth code is sent back to the server, which it uses to exchange for a long-term refresh token and a short-term access token. The access token is returned to the client.
  • The client initializes a gdrive picker widget with the access token.
  • The user is asked to pick a folder to give kopia access to. The folder id is then sent back to the server.
  • The token and folder id are persisted, the server is shutdown.

Edge cases:

  • If the user lingers on the picker for >1hr without doing anything, the access token expires. We don't currently handle that, the user needs to re-run the command.
  • If the refresh token expires for any reason, we need to prompt the user to re-run the initial setup.

Thanks,

xkxx

@xkxx xkxx changed the title feat: Support interactive oauth setup for gdrive. feat(providers): Support interactive oauth setup for gdrive. Sep 11, 2023
@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Attention: Patch coverage is 9.33333% with 68 lines in your changes are missing coverage. Please review.

Project coverage is 76.89%. Comparing base (cb455c6) to head (a051bb6).
Report is 49 commits behind head on master.

Files Patch % Lines
internal/gdriveauth/gdrive_auth.go 0.00% 50 Missing ⚠️
cli/storage_gdrive.go 28.00% 18 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3288      +/-   ##
==========================================
+ Coverage   75.86%   76.89%   +1.02%     
==========================================
  Files         470      477       +7     
  Lines       37301    28832    -8469     
==========================================
- Hits        28299    22169    -6130     
+ Misses       7071     4740    -2331     
+ Partials     1931     1923       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xkxx
Copy link
Contributor Author

xkxx commented Sep 29, 2023

@jkowalski Friendly ping on this.

@xkxx
Copy link
Contributor Author

xkxx commented Oct 7, 2023

Could I get a preliminary review on this PR?

@petefinesse
Copy link

@jkowalski Can we prioritize it, please? service-account quotas are confusing for everyone. There are multiple topics/slack questions about it

@jkowalski
Copy link
Contributor

Sorry I've been sitting on this for too long, I don't really use google drive myself so am not very familiar with it and this wasn't most straightforward to test.

Overall UI and the approach looks reasonable. I have some questions:

  1. we need step-by-step instructions for testing it, in particular which keys do I need to generate beforehand? Can you add a doc page?
  2. We need some kind of automated tests
  3. How do we deal with access tokens persistence? My understanding is they should be somewhat persisted/cached so we don't exchange refresh tokens for access tokens on each session.
  4. How would refresh token be extended, are those eternal or will refresh tokens expire after a while?
  5. Would it be practical to register app/client credentials and embed those in the app so that folks don't need to do all those manual steps?
  6. How would this extend to KopiaUI? How can we integrate the authorization flow into the web app we already have and the desktop app?

@xkxx
Copy link
Contributor Author

xkxx commented Mar 13, 2024

Hi @jkowalski,

Thanks for taking a look at this PR.

We need step-by-step instructions for testing it, in particular which keys do I need to generate beforehand? Can you add a doc page?

Sure, I'll work on that. I want to get the initial design out before working on the peripherals.

We need some kind of automated tests

Ditto

How do we deal with access tokens persistence? My understanding is they should be somewhat persisted/cached so we don't exchange refresh tokens for access tokens on each session.

The Oauth2 client takes care of exchanging refresh tokens for access tokens and repeats it as necessary (every hour or so).

How would refresh token be extended, are those eternal or will refresh tokens expire after a while?

Refresh tokens are forever until the user manually revokes it. If we encounter an invalid refresh token, we should ask the user to re-auth if it's an interactive session, or else fail.

Would it be practical to register app/client credentials and embed those in the app so that folks don't need to do all those manual steps?

It's technically feasible. DriveFile scope is not sensitive, so we don't have to pay for an expensive security audit.

However, we'd still reveal the api key & client secret in an open source repository or at the very least embed them into a binary, which isn't much better security-wise. People can do very nefarious things with it. The OAuth security model isn't designed for this.

How would this extend to KopiaUI? How can we integrate the authorization flow into the web app we already have and the desktop app?

That's what I want to discuss with you! Is there a way to embed the auth server and client code into the existing React app? That would make the Google Drive onboarding seamless for GUI users.

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.

[design] Migrate GDrive from service account to OAuth
3 participants