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

refreshAccessToken gets new refreshToken #22

Open
kevinsherman opened this issue Mar 8, 2021 · 4 comments
Open

refreshAccessToken gets new refreshToken #22

kevinsherman opened this issue Mar 8, 2021 · 4 comments

Comments

@kevinsherman
Copy link
Contributor

In src/client/resources/tokens.js there is a refreshAccessToken function. The function is intended to use an unexpired refreshToken to refresh an expired accessToken. In the function the parameter 'access_type' is being set to 'offline'. Per the documentation, "Set to offline to receive a refresh token on an authorization_code grant type request. Do not set to offline on a refresh_token grant type request."

My understanding is that this should only be set to offline when requesting the initial tokens (with the 'code' from the redirectURI).

When tested with 'offline' - this it returns both a refresh and access token.
When tested without 'offline' - this ONLY returns an access token (leaving the unexpired refreshToken still valid).

I will put together a PR for this change.

@knicola
Copy link
Owner

knicola commented Mar 8, 2021

Hello, thank you for taking the time to contribute! This behavior is controlled by the config.accessType setting. Also, though the documentation specifically says not to use offline on a refresh_token request, it is still a valid request, as far as I can tell, and it helps avoid the complexity of deciding when to refresh an access token and when a refresh token since both are refreshed at the same time. Despite that, I'll be happy to move things around if it makes better sense. I'll review the PR hopefully by the eod.

@marknokes
Copy link
Contributor

I've thought about this behavior a little too, but what's really been on my mind lately is this; I've been logging a message inside the "on token" event td.on('token', async token => {} and notice that it can fire many times in a row. Is there some way to refresh the token and wait until it's finished before exiting? If so, subsequent calls to the "is expired" methods would return false, preventing additional calls to refresh.

@knicola
Copy link
Owner

knicola commented Apr 28, 2022

@marknokes that sounds like a race condition in which case a lock would be a nice thing to have. we'll first need a test to recreate the issue. would you like to give it a go?

@marknokes
Copy link
Contributor

I'm not sure I would know where to start, tbh. I found the refreshAndRetry method, and thought it could perhaps be the culprit. I don't really know how to test it though.

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

No branches or pull requests

3 participants