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

Token refresh time is likely set too "short" if less than 60 seconds left before it expires. #272

Closed
GaryAustin1 opened this issue Apr 14, 2022 · 2 comments · Fixed by #278
Labels
bug Something isn't working p3 Priority 3 released

Comments

@GaryAustin1
Copy link

GaryAustin1 commented Apr 14, 2022

Bug report

In looking into another issue I found this code in gotrue.js:

  if (expiresAt) {
      const timeNow = Math.round(Date.now() / 1000)
      const expiresIn = expiresAt - timeNow
      const refreshDurationBeforeExpires = expiresIn > 60 ? 60 : 0.5
      this._startAutoRefreshToken((expiresIn - refreshDurationBeforeExpires) * 1000)

This code was modified in June 21 here: #101 to deal with jwt expire set to less than 60 seconds.

This comment does not seem valid:
"0.5s is chosen to support JWT expiry of 1s, even though that should be an edge case. That should usually still be enough time to reach the server before expiry."

image

This is from Texas to us-east instance and the request are slightly over to slightly less than 500msec in just one random sample today for CORS with preflight requests.

If I login the client with less than 61 seconds on my token, refresh will be set to occur 1 (for 61) to .5 seconds before the token expires. If I issue a request right as the token is getting ready to expire, we are at the mercy of https response getting a new token back into local storage within .5 seconds or there is a window of invalid token.

I agree it is a small window, and would appear to be a random error in the scheme of things.
Edit: Also if a realtime subscription is running it could lose subscription and have to reconnect if heart beat occurs in the window

To Reproduce

This is a concern at this point based on looking at the code and token refresh times.

Expected behavior

Token should be refreshed with plenty of time for round trip of https requests. This may mean it is unrealistic to support token expiration setting of 5 (or some number) or less.

@GaryAustin1 GaryAustin1 added the bug Something isn't working label Apr 14, 2022
@GaryAustin1 GaryAustin1 changed the title Token refresh time is likely set to short if less than 60 seconds left before it expires. Token refresh time is likely set too short if less than 60 seconds left before it expires. Apr 14, 2022
@GaryAustin1 GaryAustin1 changed the title Token refresh time is likely set too short if less than 60 seconds left before it expires. Token refresh time is likely set too "short" if less than 60 seconds left before it expires. Apr 15, 2022
@GaryAustin1
Copy link
Author

GaryAustin1 commented Apr 19, 2022

Adding further. If _recoverAndRefresh() in gotrue.js is important on timing, because of rounding down it can report a valid token when already expired...

Edit. I believe a similar problem also exists in _recoverSession(), but did not play with calculations...

image
This I believe expiresAt<timeNow yields:

expiresAt       timeNow      Date.now()    
xxx1               xxx0             xxx0.499            "good" 1/2 sec to expire left  
xxx1               xxx1             xxx0.999            "good" .001 sec to expire!
xxx1               xxx1             xxx1.499            "bad" expired 1/2 sec ago but treated as not expired
xxx1               xxx2             xxx1.501             good it is expired for real

@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2022

🎉 This issue has been resolved in version 1.22.14 🎉

The release is available on:

Your semantic-release bot 📦🚀

@egor-romanov egor-romanov added the p3 Priority 3 label Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p3 Priority 3 released
Projects
None yet
2 participants