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

Support refresh token syncing for multiple tabs #444

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 93 additions & 6 deletions src/GoTrueClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import type {
SignUpWithPasswordCredentials,
Subscription,
SupportedStorage,
SyncTokenRefreshStorage,
User,
UserAttributes,
UserResponse,
Expand Down Expand Up @@ -71,6 +72,11 @@ export default class GoTrueClient {
*/
protected storageKey: string

/**
* The storage key used to keep the refresh token synced with tabs
*/
protected refreshTokenSyncStorageKey: string

/**
* The session object for the currently logged in user. If null, it means there isn't a logged-in user.
* Only used if persistSession is false.
Expand All @@ -82,6 +88,8 @@ export default class GoTrueClient {
protected storage: SupportedStorage
protected stateChangeEmitters: Map<string, Subscription> = new Map()
protected refreshTokenTimer?: ReturnType<typeof setTimeout>
protected refreshTokenSyncIntervalTimer?: ReturnType<typeof setTimeout>
protected refreshTokenSyncTimeoutTimer?: ReturnType<typeof setTimeout>
protected networkRetries = 0
protected refreshingDeferred: Deferred<CallRefreshTokenResult> | null = null
/**
Expand All @@ -105,6 +113,7 @@ export default class GoTrueClient {
const settings = { ...DEFAULT_OPTIONS, ...options }
this.inMemorySession = null
this.storageKey = settings.storageKey
this.refreshTokenSyncStorageKey = `${this.storageKey}-rts`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to actually storage this information in the same storage used for the session?

this.autoRefreshToken = settings.autoRefreshToken
this.persistSession = settings.persistSession
this.storage = settings.storage || localStorageAdapter
Expand Down Expand Up @@ -822,24 +831,63 @@ export default class GoTrueClient {
}

try {
let syncedSession: Session | null = null
const refreshTokenSync = (await getItemAsync(
this.storage,
this.refreshTokenSyncStorageKey
)) as SyncTokenRefreshStorage | undefined

if (
refreshTokenSync?.refresh_token === refreshToken &&
refreshTokenSync?.status === 'TOKEN_REFRESHING'
) {
await this._waitOnRefreshTokenSync()
const latestSession = (await getItemAsync(this.storage, this.storageKey)) as Session | null
if (this._isSessionValid(latestSession)) {
syncedSession = latestSession
}
} else if (refreshTokenSync?.status === 'TOKEN_REFRESHED') {
const latestSession = (await getItemAsync(this.storage, this.storageKey)) as Session | null
if (this._isSessionValid(latestSession)) {
syncedSession = latestSession
}
}

this.refreshingDeferred = new Deferred<CallRefreshTokenResult>()

if (!refreshToken) {
throw new AuthSessionMissingError()
}
const { data, error } = await this._refreshAccessToken(refreshToken)
if (error) throw error
if (!data.session) throw new AuthSessionMissingError()

await this._saveSession(data.session)
this._notifyAllSubscribers('TOKEN_REFRESHED', data.session)
if (!syncedSession) {
// Cache refresh token status for other tabs
await setItemAsync(this.storage, this.refreshTokenSyncStorageKey, {
refresh_token: refreshToken,
status: 'TOKEN_REFRESHING',
})

const { data, error } = await this._refreshAccessToken(refreshToken)
if (error) throw error
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking closer into this case when an error occurs with the request.
So far with a network disconnection, calls resolve appropriately.
I just want to make sure everything is well accounted for, so just sharing what's next on my list to validate

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this.refreshTokenSyncStorageKey is removed from storage when an error is thrown.
I tested this case using network disconnection. If the request failed we'll delete the refresh token sync status.
This will have a domino effect on all tabs:

  • Tabs finish waiting on TOKEN_REFRESHING status to change.
  • Tabs will attempt to make a network request. It will fail
  • Once one tab makes a successful network request. The tab waits. If successful avoid network request and use the latestSession

if (!data.session) throw new AuthSessionMissingError()
syncedSession = data.session

await setItemAsync(this.storage, this.refreshTokenSyncStorageKey, {
refresh_token: syncedSession.refresh_token,
status: 'TOKEN_REFRESHED',
})
}

const result = { session: data.session, error: null }
await this._saveSession(syncedSession)
this._notifyAllSubscribers('TOKEN_REFRESHED', syncedSession)
Copy link
Author

@miguelespinoza miguelespinoza Sep 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally thought about setting the refreshToken sync status inside an onAuthStateChange listener.
But I didn't see an example of this inside this class, so figured just to leave it as is. The idea required:

  • creating a listener for TOKEN_REFRESHING and TOKEN_REFRESHED in the constructor. In there call setItemAsync({..., status: "TOKEN_REFRESHING"})
  • Adding TOKEN_REFRESHING to type AuthChangeEvent

Instead, I went with the change localized to _callRefreshToken open to suggestions

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this implementation is fine, imo TOKEN_REFRESHING seems like an implementation detail and we don't have to add it to the AuthChangeEvent type.


const result = { session: syncedSession, error: null }

this.refreshingDeferred.resolve(result)

return result
} catch (error) {
await removeItemAsync(this.storage, this.refreshTokenSyncStorageKey)

if (isAuthError(error)) {
const result = { session: null, error }

Expand All @@ -855,6 +903,36 @@ export default class GoTrueClient {
}
}

private async _waitOnRefreshTokenSync(): Promise<void> {
return new Promise((resolve) => {
this.refreshTokenSyncIntervalTimer = setInterval(async () => {
const refreshTokenSync = (await getItemAsync(
this.storage,
this.refreshTokenSyncStorageKey
)) as SyncTokenRefreshStorage | undefined

if (refreshTokenSync?.status === 'TOKEN_REFRESHED') {
clearInterval(this.refreshTokenSyncIntervalTimer)
clearTimeout(this.refreshTokenSyncTimeoutTimer)
resolve()
}
}, 100)

// Stop interval if tokenSync.status does not change
this.refreshTokenSyncTimeoutTimer = setTimeout(async () => {
await removeItemAsync(this.storage, this.refreshTokenSyncStorageKey)
clearInterval(this.refreshTokenSyncIntervalTimer)
resolve()
}, 1500)
})
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw there's an exponential backoff for network retry.
I didn't think it was required here. A request to refresh a token takes on average 150-300ms.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @miguelespinoza, just to clarify, the purpose of refreshTokenSyncIntervalTimer is to continue check the storage to see if the status has changed from TOKEN_REFRESHING to ``TOKEN_REFRESHED` right?

And we have refreshTokenSyncTimeoutTimer to eventually cleanup the setInterval. At this point, if the callback in setTimeout is invoked, the status should still be TOKEN_REFRESHING right?


private _isSessionValid(session: Session | null): boolean {
return session?.expires_at
? session.expires_at - (EXPIRY_MARGIN + 0.5) >= Date.now() / 1000
: false
}

private _notifyAllSubscribers(event: AuthChangeEvent, session: Session | null) {
this.stateChangeEmitters.forEach((x) => x.callback(event, session))
}
Expand Down Expand Up @@ -888,13 +966,22 @@ export default class GoTrueClient {
private async _removeSession() {
if (this.persistSession) {
await removeItemAsync(this.storage, this.storageKey)
await removeItemAsync(this.storage, this.refreshTokenSyncStorageKey)
} else {
this.inMemorySession = null
}

if (this.refreshTokenTimer) {
clearTimeout(this.refreshTokenTimer)
}

if (this.refreshTokenSyncIntervalTimer) {
clearInterval(this.refreshTokenSyncIntervalTimer)
}

if (this.refreshTokenSyncTimeoutTimer) {
clearTimeout(this.refreshTokenSyncTimeoutTimer)
}
}

/**
Expand Down
5 changes: 5 additions & 0 deletions src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,11 @@ type PromisifyMethods<T> = {
: T[K]
}

export interface SyncTokenRefreshStorage {
refresh_token: string
status: 'TOKEN_REFRESHING' | 'TOKEN_REFRESHED'
}

export type SupportedStorage = PromisifyMethods<Pick<Storage, 'getItem' | 'setItem' | 'removeItem'>>

export type InitializeResult = { error: AuthError | null }
Expand Down