Skip to content

Commit c4fd3c8

Browse files
committed
fix: resolve race conditions when refreshing access token
1 parent d7b334a commit c4fd3c8

File tree

2 files changed

+56
-3
lines changed

2 files changed

+56
-3
lines changed

src/GoTrueClient.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -615,11 +615,31 @@ export default class GoTrueClient {
615615
}
616616
}
617617

618+
private _callRefreshTokenPromises: {
619+
[refresh_token: string]: ReturnType<GoTrueClient['_callRefreshTokenConcurrently']>
620+
} = {}
621+
622+
// If there is an ongoing request a promise resolving to it will be returned.
623+
//
624+
// This is necessary because concurrent token refreshes will race and inevitably fail:
625+
// - earliest request will revoke current session and start a new one
626+
// - later requests will fail as they were sent with an old session
627+
// - this will cause new session to also be revoked due to refresh token reuse detection
628+
// - the client will be left with a valid access_token but revoked refresh_token
629+
// - they might not notice it until they try to refresh the token
618630
private async _callRefreshToken(refresh_token = this.currentSession?.refresh_token) {
631+
if (!refresh_token) {
632+
return { data: null, error: { message: 'No current session', status: 401 } as ApiError }
633+
}
634+
if (!(refresh_token in this._callRefreshTokenPromises))
635+
this._callRefreshTokenPromises[refresh_token] = this._callRefreshTokenConcurrently(
636+
refresh_token
637+
).finally(() => delete this._callRefreshTokenPromises[refresh_token])
638+
return this._callRefreshTokenPromises[refresh_token]
639+
}
640+
641+
private async _callRefreshTokenConcurrently(refresh_token: string) {
619642
try {
620-
if (!refresh_token) {
621-
throw new Error('No current session.')
622-
}
623643
const { data, error } = await this.api.refreshAccessToken(refresh_token)
624644
if (error) throw error
625645
if (!data) throw Error('Invalid session data.')

test/GoTrueClient.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,39 @@ describe('GoTrueClient', () => {
8787
expect(data).toHaveProperty('refresh_token')
8888
expect(refreshToken).not.toEqual(data?.refresh_token)
8989
})
90+
91+
test('refreshSession() can be called concurrently', async () => {
92+
const { email, password } = mockUserCredentials()
93+
94+
const { error: initialError, session } = await authWithSession.signUp({
95+
email,
96+
password,
97+
})
98+
99+
expect(initialError).toBeNull()
100+
expect(session).not.toBeNull()
101+
102+
const initialRefreshToken = session?.refresh_token
103+
104+
const refreshSessionA = authWithSession.refreshSession()
105+
const refreshSessionB = authWithSession.refreshSession()
106+
await new Promise((resolve) => setTimeout(resolve, 0))
107+
const refreshSessionC = authWithSession.refreshSession()
108+
109+
const sessionC = await refreshSessionC
110+
const sessionB = await refreshSessionB
111+
const sessionA = await refreshSessionA
112+
113+
for (const { error, user, data } of [sessionA, sessionB, sessionC]) {
114+
expect(error).toBeNull()
115+
expect(user).not.toBeNull()
116+
expect(data).not.toBeNull()
117+
118+
expect(user?.email).toEqual(email)
119+
expect(data).toHaveProperty('refresh_token')
120+
expect(initialRefreshToken).not.toEqual(data?.refresh_token)
121+
}
122+
})
90123
})
91124

92125
describe('Authentication', () => {

0 commit comments

Comments
 (0)