dropbox-sdk-rust icon indicating copy to clipboard operation
dropbox-sdk-rust copied to clipboard

Obtain Access Token seems to fail

Open pvichivanives opened this issue 9 months ago • 7 comments

What is your question? Not sure if I'm missing something or theres a bug with the code but I am unable to fetch access_tokens from the refresh token. I am able to create the Authorization State of refresh token from initialAuth but unable to proceed further.

Screenshots If applicable, add screenshots to help explain your question. For this image, we would get an auth code from the login with details from the user and my client secret. The first call would go successfully, going from the state of InitialAuth to RefreshToken. However, any call with the refresh token to get an access_code again (like the second call here) would fail with the error: Dropbox API returned HTTP 400 Bad Request - {"error": "invalid_request", "error_description": "No auth function available for given request"}. image

Versions

  • What version of the SDK are you using? 0.18.0
  • What version of the language are you using? 1.77.2
  • What platform are you using? Mac OSX

Additional context For reference this is how I generate the initial dropbox auth request although since it got through the first initialAuth I would assume this isn't the issue image

pvichivanives avatar May 01 '24 23:05 pvichivanives

Can you share what is the Oauth2Type you're using? It's the second parameter to Authorization::from_auth_code(). Redact any secrets of course.

wfraser avatar May 02 '24 18:05 wfraser

Hi! I'm using a client Secret with the app_secret from Dropbox. Thanks!

 let dropbox_client_secret = Oauth2Type::AuthorizationCode {
        client_secret: "secret goes here",
    };

pvichivanives avatar May 02 '24 21:05 pvichivanives

Thanks. I'm able to reproduce the problem. Apparently refresh tokens obtained using a client secret are different from ones obtained using the PKCE flow, and we need to pass the client secret along with the refresh token when obtaining a new token. This is a difference I wasn't previously aware of.

I'll make a fix to Authorization::obtain_access_token() which does the right thing here for any new authorizations.

From your side, there are a couple options. First, and maybe easiest, you can change to using PKCE instead of the client secret to do the authorization. The code for that doesn't have the bug and is basically identical in every other way. You can actually do that right now before I make a new release with this fix.

If you have existing refresh tokens that you need to keep using, you'll need to pick up the fix for this, and your code will need a slight change, since Authorization doesn't know whether it was obtained using a client secret or not. I'll be adding a new function Authorization::from_client_secret_refresh_token() which takes the client secret as well as the refresh token, and that should work for any existing tokens obtained that way.

wfraser avatar May 02 '24 21:05 wfraser

@pvichivanives do you think you could test this fix and let me know if it works for you? https://github.com/dropbox/dropbox-sdk-rust/pull/152

wfraser avatar May 02 '24 22:05 wfraser

@wfraser It works but I think you missed the save/load still needing the client_secret as well. Authorization::load will always pick from_refresh_token (tried extracting refresh token and using it in from_client_secret_refresh_token and that worked)

pvichivanives avatar May 02 '24 22:05 pvichivanives

If you're using save/load, you really should be using PKCE anyway. The alternative would require saving the secret in there too, which is not a great idea since in the case of a leak, a token can be easily revoked, but the client secret not so much.

wfraser avatar May 02 '24 22:05 wfraser

Would it make sense to have an alternate to save/ save the refresh token without the "2&" if theres a secret? This would make it possible to save refresh token but not put the secret inside and then reuse the token later with the same client id/secret later. An alternative idea would be that load could take in an Option of a secret but then that would be a breaking change.

pvichivanives avatar May 02 '24 23:05 pvichivanives

Yeah, as you mention, it'd be a breaking change. It'd also break the CLI helper get_auth_from_env_or_prompt() which assumes there are just two env variables, one with client ID and one with the saved string.

However the whole load() function just calls public functions, so if you'd like such functionality, you can roll your own easily.

wfraser avatar May 06 '24 18:05 wfraser