transloadit / uppy

The next open source file uploader for web browsers :dog:
https://uppy.io
MIT License
29.05k stars 2k forks source link

Support refresh_token #2721

Closed mejiaej closed 1 year ago

mejiaej commented 3 years ago

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?

othierry-asi commented 3 years ago

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 commented 3 years ago

Similar question. Is there a way to use refresh_token?

Murderlon commented 3 years ago

@mifi are you able to answer this?

mifi commented 3 years ago

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:

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:

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

Murderlon commented 2 years ago

@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 commented 2 years ago

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

Murderlon commented 2 years ago

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

mifi commented 1 year ago

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;
  }  
}