feat(gotrue, supabase_flutter): New auth token refresh algorithm #879
Conversation
| case AppLifecycleState.detached: | ||
| case AppLifecycleState.inactive: | ||
| case AppLifecycleState.paused: | ||
| Supabase.instance.client.auth.stopAutoRefresh(); |
There was a problem hiding this comment.
What's the reason to stop auth refresh here? From what I read in the documentation the app may still be running in the background.
There was a problem hiding this comment.
The reason is the following:
In addition to the algorithm update, this PR has introduced the start and stop auto refresh methods, and uses them with the lifecycle events of Flutter. Specifically, when the app is in the background, it stops the SDK to attempt to refresh the token. This is because when the app is in the background, HTTP requests may or may not go through and the app could be killed any time. In order to reduce the chance of something wrong happening with the token refresh requests, the refresh requests should only be fired when the app is in the foreground.
There was a problem hiding this comment.
Oh sorry, seems like I didn't read that.
But does it matter if a refresh request fails? The app could still be running, so the application could send requests to supabase with outdated access token. If the refresh fails, then requests from the application will also fail, which is fine.
There was a problem hiding this comment.
No problem at all!
The app could still be running, so the application could send requests to supabase with outdated access token. If the refresh fails, then requests from the application will also fail, which is fine.
Actually, stopAutoRefresh() only stops periodically checking the token if it's expired then refresh if expired, and if a user would to make a request after stopAutoRefresh() has been called, the auth HTTP client will still refresh the token as needed.
All it's doing is avoiding a slim chance of the auth refresh request firing, which reaches the server, but before the response comes back the connection is lost. This is where the app could be left with an expired JWT. This could very much happen when the app is in the foreground as well, but if the app is running for a long time in the background, there might be more chances of something like this happening.
It was advice from our Auth engineer and we also recommend it for React native apps. On step 4 there is pretty much the same code:
https://supabase.com/docs/guides/auth/quickstarts/react-native
There was a problem hiding this comment.
Oh yeah I forgot that the auth http client still exists. Then you are right this is fine. Thanks for the explanation.
I will one last time go through the changes now
Co-authored-by: Vinzent <vinzent03@proton.me>
Co-authored-by: Vinzent <vinzent03@proton.me>
Co-authored-by: Vinzent <vinzent03@proton.me>
Co-authored-by: Vinzent <vinzent03@proton.me>
| if (!_refreshTokenCompleter!.isCompleted) { | ||
| _refreshTokenCompleter!.completeError(error, stack); | ||
| } | ||
| _onAuthStateChangeController.addError(error, stack); |
There was a problem hiding this comment.
This is now missing from what I see. I think we should keep it when it's not retryable, because when this is called from a tick, the not retriable exception is nowhere reported.
There was a problem hiding this comment.
Great catch! I added it back!
| /// Starts an auto-refresh process in the background. Close to the time of expiration a process is started to | ||
| /// refresh the session. If refreshing fails it will be retried for as long as necessary. | ||
| /// | ||
| /// If you set the `autoRefreshToken` to `true`, you don't need to call this function, it will be called for you. |
There was a problem hiding this comment.
This is not really true. Only when using the supabase_flutter package this method is called, nowhere else. Additionally, autoRefreshToken should be respected when calling startAutoRefresh in supabase_flutter as well.
There was a problem hiding this comment.
Great catch! Pushed a fix.
There was a problem hiding this comment.
I think the auto refresh should be started when only using the supabase or gotrue package as well. Not only when using the flutter package
There was a problem hiding this comment.
Makes sense. Let me change the code right now.
There was a problem hiding this comment.
@Vinzent03 Updated it to call the startAutoRefresh from within gotrue.
What kind of change does this PR introduce?
Change the way we refresh the auth tokens. It also creates two new auth methods
startAutoRefresh()andstopAutoRefresh()to start and stop the token refresh process.What is the current behavior?
We have a relatively complex algorithm to refresh the auth tokens. It can be hard to debug what is happening through out the token refresh flow.
What is the new behavior?
The flow is simplified, and easier to understand.
CompleterIn addition to the algorithm update, this PR has introduced the start and stop auto refresh methods, and uses them with the lifecycle events of Flutter. Specifically, when the app is in the background, it stops the SDK to attempt to refresh the token. This is because when the app is in the background, HTTP requests may or may not go through and the app could be killed any time. In order to reduce the chance of something wrong happening with the token refresh requests, the refresh requests should only be fired when the app is in the foreground.
Additional context
Closes #317
Related #171
Related #788
Related #827
Closes #829
Related #860
Closes #872