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

Support refresh_token #2721

Closed
mejiaej opened this issue Jan 5, 2021 · 8 comments · Fixed by #4448
Closed

Support refresh_token #2721

mejiaej opened this issue Jan 5, 2021 · 8 comments · Fixed by #4448
Labels
Companion The auth server (for Instagram, GDrive, etc) and upload proxy (for S3) Feature

Comments

@mejiaej
Copy link
Contributor

mejiaej commented Jan 5, 2021

Hi, I haven't found any documentation related to the expiration of tokens in a google drive upload scenario. I want to know how often a user is gonna need to consent. Google OAuth gives an acces_token which is short lived and a refresh_token with an expiration of 6 months on production.

I haven't found any reference to refresh tokens inside companion, Does companion use refresh_tokens? If not how does companion handle the expiration of short-lived access_token?

@arturi arturi added the Companion The auth server (for Instagram, GDrive, etc) and upload proxy (for S3) label Jan 20, 2021
@othierry-asi
Copy link

othierry-asi commented May 28, 2021

I have the same question. Is there a way to have a long authentication between the companion provider and the cloud provider ? What I want to do is ask permissions to the user once, then store the refresh_token and use it further so the user does not need to authenticate every time it wants to upload a file from Dropbox or any storage provider.

@dovydaskukalis
Copy link

Similar question. Is there a way to use refresh_token?

@Murderlon
Copy link
Member

@mifi are you able to answer this?

@mifi
Copy link
Contributor

mifi commented Oct 1, 2021

Hi, I haven't found any documentation related to the expiration of tokens in a google drive upload scenario. I want to know how often a user is gonna need to consent. Google OAuth gives an acces_token which is short lived and a refresh_token with an expiration of 6 months on production.

Hi. As far as I know (from my testing), the user does not receive a new consent dialog every time the access token is expired, but they just need to reauthenticate. I believe this is because they have already allowed the app to access their account so they just need to choose their google account again (and possibly enter password again).

You are right that the access token lasts a short time (e.g. less than an hour or so) and this is by design / best practice. And companion does not support refresh tokens, and so the user will have to reauthenticate after the short lived access token has expired.

So I consider this a feature request.

Implementation

Refreshing the token

If we were using Google's official Node.js SDK it would automatically handle refreshing the token:

Access tokens expire. This library will automatically use a refresh token to obtain a new access token if it is about to expire. An easy way to make sure you always store the most recent tokens is to use the tokens event:

https://github.com/googleapis/google-api-nodejs-client#handling-refresh-tokens

But because we are using a custom HTTP client (purest), we need to either

Storing the updated refresh token

Upon receiving a refreshed token from google we need to do this:

  • replace the access_token and refresh_token inside the JWT with the new ones, repack and encrypt the JWT
  • notifiy the client about the new encrypted jwt (maybe a response header on the request's response)
  • the client would then need to store the new encrypted jwt so that it will use that in subsequent requests
  • must still handle the error if a refresh token has expired or been invalidated

refresh_token/access_token race condition

Need to look into possible race condition:

If there are multiple simultaneous companion requests that all refresh the tokens at the same time, will that "just work"? (can a single refresh_token be refreshed many times and produce many new valid refresh_tokens and access_tokens? or will only the first refresh call succeed, and the rest of the requests will fail, leading to potentially many failed uploads for the user?

I found this:

In multiprocess or distributed apps, sharing the credential requires you to persist it. To ensure multiple processes or servers don't attempt to refresh the credential at the same time (resulting in excessive refresh requests), we advise proactively refreshing the credential in a central place and sharing it across processes/servers.

Possible solutions:

  • Proactively refresh the token before it expires and update the client. Old access token should still work for a while, so any other requests should go through. Need to somehow make sure that only one request/server will do this refresh operation. I'm not sure how to best do this.
  • Use a request queue on the client or server so that only one http request (per encrypted jwt) can be sent to google at any given time. (e.g. https://github.com/sindresorhus/p-queue)
    • harder to implement on the server (needs to be enforced across multiple horizontally scaled companion instances.)

This also needs to be implemented for all the other companion providers.

@mifi mifi changed the title What's the companion Google drive token expiration time? Support refresh_token Oct 1, 2021
@Murderlon
Copy link
Member

@mifi thanks for the elaborate research. From the looks of it, this is a lot of effort to implement for questionable gains so I'm inclined to close this for that reason. We need to start prioritizing what we will actually work on at some point, and be real about it when we won't.

cc @arturi

@mifi
Copy link
Contributor

mifi commented Dec 6, 2021

yea, it adds another level of complexity, and refresh tokens differ between all the providers.

@Murderlon
Copy link
Member

Alright. Closing this but may revisited and reopened at some point.

@mifi
Copy link
Contributor

mifi commented Apr 18, 2023

reconsidering this - maybe we should do the refreshing of the token in the Uppy client - then we don't need to coordinate multiple refreshing the token between companion servers, but instead we could do it in the client, because there's always just one client.

We would still have to coordinate this between all ongoing requests and RateLimitQueue.

Let's reopen this, because it will effectively prevent any upload sessions longer than about 1 hour.

pseudo code:

// global
let refreshingTokenPromise;

async function downloadFile() {
  try {
    if (refreshingTokenPromise) await refreshingTokenPromise;

    await get('/companion/files/12345')
  } catch (err) {
    if (err.status === 401) {
      refreshingTokenPromise = put('/companion/authtoken/refresh', { refreshToken: getRefreshToken() })
      await refreshingTokenPromise
      refreshingTokenPromise =  undefined
      return downloadFile()
    }
    throw err;
  }  
}

@mifi mifi reopened this Apr 18, 2023
mifi added a commit that referenced this issue May 12, 2023
for dropbox and google drive

closes #2721
mifi added a commit that referenced this issue Jun 22, 2023
* allow storing multiple tokens

inside uppy auth token

* de-duplicate uploadRemote

by creating a new superclass UploaderPlugin

* pull out requestSocketToken

from MiniXHRUpload

* add class UploaderPlugin

* refactor

* refactor

* refactor/reuse

* refactor/make getAuthToken private

* fix bug

* implement refresh token

for dropbox and google drive

closes #2721

* fix test

* also update auth token cookie
when refreshing token

* fix build error on node 14

* increase auth token expiry

to workaround expiry

* Update packages/@uppy/companion-client/src/RequestClient.js

Co-authored-by: Antoine du Hamel <[email protected]>

* make queueRequestSocketToken private

* rename arg

* fix lint

* log error message

* fix s3

* Update packages/@uppy/companion-client/src/Provider.js

Co-authored-by: Antoine du Hamel <[email protected]>

---------

Co-authored-by: Antoine du Hamel <[email protected]>
mifi added a commit that referenced this issue Sep 1, 2023
mifi added a commit that referenced this issue Sep 6, 2023
* handle google drive refresh token revoked

* implement onedrive refresh tokens #2721
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Companion The auth server (for Instagram, GDrive, etc) and upload proxy (for S3) Feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants