spatie / flysystem-dropbox

A flysystem driver for Dropbox that uses the v2 API
https://freek.dev/734-dropbox-will-turn-off-v1-of-their-api-soon-its-time-to-update-your-php-application
MIT License
342 stars 50 forks source link

Dropbox Refresh token needed - Long Lived Access Tokens Depecrated #96

Open mho22 opened 9 months ago

mho22 commented 9 months ago

Prior to : #86 and #95

I wanted to ask you if a new PR was necessary now that Dropbox has an updated way to work with tokens. It needs to generate a Refresh token beforehand. I added my way of working in the issue 86.

Since mid 2021, it is now mandatory to use the dropbox refresh token process :

In the past, the Dropbox API used only long-lived access tokens. 
These are now deprecated, but will remain available as an option in the Developer console for compatibility until mid 2021.

I tried something else personally, without using AutoRefreshingDropBoxTokenService [ not necessary when using the flysystem-dropbox ] and it is less verbose :

You will still need to authorize the access to the Dropbox App using this link :

https://www.dropbox.com/oauth2/authorize?client_id<YOUR_APP_KEY>&response_type=code&token_access_type=offline

This will give you the authorization_code needed to retrieve a refresh_token with this curl request :

curl https://api.dropbox.com/oauth2/token -d code=<ACCESS_CODE> -d grant_type=authorization_code -u <APP_KEY>:<APP_SECRET>

Giving you access to the refresh_token needed in the DROPBOX_REFRESH_TOKEN indicated below . The other elements are available in your Dropbox App.

.env.example

DROPBOX_APP_KEY=
DROPBOX_APP_SECRET=
DROPBOX_REFRESH_TOKEN=
DROPBOX_TOKEN_URL=https://${DROPBOX_APP_KEY}:${DROPBOX_APP_SECRET}@api.dropbox.com/oauth2/token

config/filesystems.php

'dropbox' => [
    'driver' => 'dropbox',
    'token_url' => env('DROPBOX_TOKEN_URL'),
    'refresh_token' => env('DROPBOX_REFRESH_TOKEN'),
],

app/Providers/DropboxServiceProvider.php

use Illuminate\Contracts\Foundation\Application;
use Illuminate\Filesystem\FilesystemAdapter;
use Illuminate\Support\Facades\Storage;
use Illuminate\Support\ServiceProvider;
use League\Flysystem\Filesystem;
use Spatie\Dropbox\Client as DropboxClient;
use Spatie\FlysystemDropbox\DropboxAdapter;
use GuzzleHttp\Client;

class DropboxServiceProvider extends ServiceProvider
{
    public function boot(): void
    {
        Storage::extend( 'dropbox', function( Application $app, array $config )
        {
            $resource = ( new Client() )->post( $config[ 'token_url' ] , [
                    'form_params' => [ 
                            'grant_type' => 'refresh_token', 
                            'refresh_token' => $config[ 'refresh_token' ] 
                    ] 
            ]);

            $accessToken = json_decode( $resource->getBody(), true )[ 'access_token' ];

            $adapter = new DropboxAdapter( new DropboxClient( $accessToken ) );

            return new FilesystemAdapter( new Filesystem( $adapter, $config ), $adapter, $config );
        });
    }
}

A PR in code and documentation should be interesting. Should I PR this ? The previous code is also indicated in the Laravel 10 Documentation - Custom Filesystems

xmontero commented 6 months ago

Hi, there!

I'm trying FlySystem with multiple adapters, one of those being dropbox, to access server-side offline-mode some directories.

2 days ago I tried it for the first time and managet it to work, but run into the 4-hour problem, as I made a "token in the web" which ends up being a short-lived access-token.

Yesterday and today I've carefully read all this documentation:

Once finished reading I wanted to see if the flysystem-dropbox adapter supported re-generating the access-tokens from a refresh token.

I have searched for refresh in the repo and I've seen this has been discussed/proposed since 2022 with solutions that have not yet been finally integrated in the library, despite there are attempts to move on and do a PR.

My interest

My interest is making it to reach offline access for certain dropbox accounts we control (no "final user implication"). Say I need a server process that every hour needs to generate a report and make it available in a storage like dropbox. No "third users" implied in my use-case. I just don't want it to start failing after 4 hours.

Questions

a) Am I correct? Is this server-side offline-mode no-user-interaction-mode still pending to be implemented in the flysystem-dropbox adapter? If already implemented where I can see docs about how to consume it? b) If still pending: may I contribute? (I've seen many of you are laravel-oriented, I'd test it within symfony, instead).

mho22 commented 6 months ago

@xmontero In my situation, when I need to back up a project daily, the code I provided above functions well, and I haven't encountered any issues with the '4-hour problem.' Does it also work for you when you follow the steps I outlined earlier?

xmontero commented 6 months ago

Hey @mho22 thanks

I don't know if in your class, that Illuminate-something is dependent on the Laravel framework. I can't recognize those use. Googling for Illuminate\Contracts\Foundation\Application it seems so, so I feel your code might potentially have high-couplings.

I have done a similar solution inside my project. But I wanted my code to be designed to be framework-independent. I have all "dependency injected" so it could be used and reused within any framework using the flysystem-dropbox.

Here's what I did: I've created a class, in my case App\Infrastructure\Flysystem\DropboxApiAdapter with just one single method: getAccessToken(). This class is completely separate and independent from any class in this library and it is used exclusively to get and cache an access-token.

From the consumption point of view it's very simple: In my own FlySystemFactory I had this:

public function __construct(
    private string $accessToken,
)
{
}

public function createFilesystem() : Filesystem
{
    $dropboxClient = new DropboxClient( $this->accessToken );
    $adapter = new DropboxAdapter( $dropboxClient );
    $filesystem = new Filesystem( $adapter, [ 'case_sensitive' => false ] );
    return $filesystem;
}

and I change it into this:

public function __construct(
    private DropboxApiAdapter $dropboxApiAdapter,
)
{
}

public function createFilesystem() : Filesystem
{
    $accessToken = $this->dropboxApiAdapter->getAccessToken();
    $dropboxClient = new DropboxClient( $accessToken );
    $adapter = new DropboxAdapter( $dropboxClient );
    $filesystem = new Filesystem( $adapter, [ 'case_sensitive' => false ] );
    return $filesystem;
}

The DropboxApiAdapter (which is the class similar to yours in the sense that it calls the /oauth2/token endpoint) gets a cache object implementing the PSR cache interface, so again I don't care "where" to store the gotten token.

I don't mind, in my implementation, if one wants to store the access-token to be reused during consecutive calls for 3-4 hours in the filesystem, in a database, in memory or what... I just "cache it" via the cache interface.

The method DropboxApiAdapter::getAccessToken() just tries to get any existing token from the cache. In the case the access token is not cached, and only in that case, then it makes a real call to the https://api.dropboxapi.com/oauth2/token call, very similar to yours (passing-in the refresh_token as well as app key and secret in the form_params of a guzzle-based client).

The gotten token is then stored into the cache for less time than its expiration in dropbox; so it'll be wiped from the cache before giving a 401 at any API call, so a new token is generated from time to time and re-cached again. At this moment I've hardcoded 10 mins although the proper way would be to read the duration from the API json reponse and take that value and substract a few minutes.

The cache survives many controller calls, so the API to get the token is only called from time to time. In my current dirty implementation, once every 10 mins. It could be improved, of course.

So, in short: I have it really solved in my application. I just wanted to leverage the library if this functionality was shifted inside it at some point of time and delete my implementation from my code, or offer me to push code to the library if it helps.

I may share my class if anyone is interested.

mho22 commented 6 months ago

@xmontero Okay, let's wait for a response from Spatie then!

BoweFrankema commented 4 months ago

@xmontero do you still use this logic and I'd be super interested in the class if you're willing to share!

sfinktah commented 3 months ago

@mho22 This looks like a very confusing way to do something rather simple. Why not just do:

$client = new Client([$appKey, $appSecret]);
$adapter = new DropboxAdapter($client);
mho22 commented 3 months ago

@sfinktah Have you tried this over a longer period? Long-lived access tokens are no longer generated, so your tokens will eventually expire. The method suggested above is a way to keep your tokens 'alive' by refreshing them while connecting to the Dropbox filesystem. At least, this was still the case six month ago.

xmontero commented 3 months ago

@xmontero do you still use this logic and I'd be super interested in the class if you're willing to share!

Sorry, seen late. Still interested? Or am I late to the party? I'd have to dedicate time to find the class, etc. so will share only if it's going to add value.

BoweFrankema commented 3 months ago

If you have it laying around somewhere it would still be super useful! I do think ideally it would be documented/added to core, but I've kind of held off with adding this to our product due to being unsure how to implement it!

sfinktah commented 2 weeks ago

@mho22

@sfinktah Have you tried this over a longer period? Long-lived access tokens are no longer generated, so your tokens will eventually expire. The method suggested above is a way to keep your tokens 'alive' by refreshing them while connecting to the Dropbox filesystem. At least, this was still the case six month ago.

Well, I just tried it then, and while there are no errors produced, the directory listing shows no files. I generated a new token, and the result is the same.

I don't know what to say. Very annoying.