-
-
Notifications
You must be signed in to change notification settings - Fork 181
Description
Bug report
- I confirm this is a bug with Supabase, not with my own application.
- I confirm I have searched the Docs, GitHub Discussions, and Discord.
Describe the bug
When multiple processes share the same auth local storage (keychain in an app group, for example) and they try to refresh the token at the same time, one of them succeeds, and the others fail since the refresh token was already used, then the SDK deletes the token from the keychain.
It is similar to what was reported here:
#486
To Reproduce
This issue happens with any configuration when there are multiple processes involved sharing the same auth local storage and they try to refresh the token at the same time. This is hard to reproduce, but eventually the configuration below will reproduce the issue.
- Create a custom auth local storage with a keychain in an app group.
- Run different processes using the shared keychain. For example, a single app and an app extension, or two apps. Whatever combination of different processes using the same keychain. In my case it was an app, a widget and app intents.
Expected behavior
It is expected that the SDK doesn't delete valid tokens from the keychain.
Proposed Fix
My fix proposal is to introduce a check before deleting tokens. If the error that was thrown in the api.execute
call of LiveSessionManager.refreshsession(_ refreshToken:)
is AuthError.api
where errorCode
is "refresh_token_already_used"
, then extract the current session from storage, if the session is not expired, do nothing.
supabase-swift/Sources/Auth/Internal/SessionManager.swift
Lines 100 to 116 in 5c06db6
} catch { | |
logger?.debug("Refresh token failed with error: \(error)") | |
// DO NOT remove session in case it is an error that should be retried. | |
// i.e. server instability, connection issues, ... | |
// | |
// Need to do this check here, because not all RetryableError's should be retried. | |
// URLError conforms to RetryableError, but only a subset of URLError should be retried, | |
// the same is true for AuthError. | |
if let error = error as? URLError, error.shouldRetry { | |
throw error | |
} else if let error = error as? any RetryableError, error.shouldRetry { | |
throw error | |
} else { | |
remove() | |
throw error | |
} |
Another approach is to not even check if the error is "refresh_token_already_used"
, maybe just never delete valid tokens in:
sessionStorage.delete() |
Screenshots
Not applicable.
System information
- This issue happens with any system that uses a shared auth local storage
Additional context
No additional context.