fix(auth): coordinate token refresh across pages

spx-gui/src/stores/user/signed-in.test.ts

@@ -0,0 +1,147 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+const userStateStorageKey = 'builder-user'
+
+describe('ensureAccessToken', () => {
+  let ensureAccessToken: typeof import('./signed-in').ensureAccessToken
+  let initUserState: typeof import('./signed-in').initUserState
+  let accountOAuthApisForXBuilder: typeof import('@/apis/account/oauth').accountOAuthApisForXBuilder
+
+  beforeEach(async () => {
+    vi.resetModules()
+    localStorage.clear()
+    ;({ ensureAccessToken, initUserState } = await import('./signed-in'))
+    ;({ accountOAuthApisForXBuilder } = await import('@/apis/account/oauth'))
+  })
+
+  afterEach(() => {
+    vi.restoreAllMocks()
+  })
+
+  it('uses credentials refreshed by another page while waiting for the refresh lock', async () => {
+    localStorage.setItem(
+      userStateStorageKey,
+      JSON.stringify({
+        accessToken: 'expired-access-token',
+        accessTokenExpiresAt: Date.now(),
+        refreshToken: 'old-refresh-token',
+        username: 'alice'
+      })
+    )
+    const request = vi.fn(async (_name: string, callback: () => Promise<void>) => {
+      localStorage.setItem(
+        userStateStorageKey,
+        JSON.stringify({
+          accessToken: 'new-access-token',
+          accessTokenExpiresAt: Date.now() + 60 * 60 * 1000,
+          refreshToken: 'new-refresh-token',
+          username: 'alice'
+        })
+      )
+      await callback()
+    })
+    Object.defineProperty(navigator, 'locks', {
+      configurable: true,
+      value: {
+        request
+      }
+    })
+    const refreshToken = vi.spyOn(accountOAuthApisForXBuilder, 'refreshToken')
+
+    initUserState('client-id')
+
+    await expect(ensureAccessToken()).resolves.toBe('new-access-token')
+    expect(request).toHaveBeenCalledWith('builder-user-access-token', expect.any(Function))
+    expect(refreshToken).not.toHaveBeenCalled()
+  })
+
+  it('returns the cached access token without acquiring the lock', async () => {
+    localStorage.setItem(
+      userStateStorageKey,
+      JSON.stringify({
+        accessToken: 'access-token',
+        accessTokenExpiresAt: Date.now() + 60 * 60 * 1000,
+        refreshToken: 'refresh-token',
+        username: 'alice'
+      })
+    )
+    const request = vi.fn(async (_name: string, callback: () => Promise<void>) => callback())
+    Object.defineProperty(navigator, 'locks', {
+      configurable: true,
+      value: {
+        request
+      }
+    })
+
+    initUserState('client-id')
+
+    await expect(ensureAccessToken()).resolves.toBe('access-token')
+    expect(request).not.toHaveBeenCalled()
+  })
+
+  it('clears cached state when the shared state is removed', async () => {
+    localStorage.setItem(
+      userStateStorageKey,
+      JSON.stringify({
+        accessToken: 'access-token',
+        accessTokenExpiresAt: Date.now() + 60 * 60 * 1000,
+        refreshToken: 'refresh-token',
+        username: 'alice'
+      })
+    )
+    Object.defineProperty(navigator, 'locks', {
+      configurable: true,
+      value: {
+        request: vi.fn(async (_name: string, callback: () => Promise<void>) => callback())
+      }
+    })
+    initUserState('client-id')
+
+    localStorage.removeItem(userStateStorageKey)
+    window.dispatchEvent(new StorageEvent('storage', { key: userStateStorageKey }))
+
+    await expect(ensureAccessToken()).resolves.toBe(null)
+  })
+
+  it('clears cached state when the shared storage is cleared', async () => {
+    localStorage.setItem(
+      userStateStorageKey,
+      JSON.stringify({
+        accessToken: 'access-token',
+        accessTokenExpiresAt: Date.now() + 60 * 60 * 1000,
+        refreshToken: 'refresh-token',
+        username: 'alice'
+      })
+    )
+    Object.defineProperty(navigator, 'locks', {
+      configurable: true,
+      value: {
+        request: vi.fn(async (_name: string, callback: () => Promise<void>) => callback())
+      }
+    })
+    initUserState('client-id')
+
+    localStorage.clear()
+    window.dispatchEvent(new StorageEvent('storage', { key: null }))
+
+    await expect(ensureAccessToken()).resolves.toBe(null)
+  })
+
+  it('does not write state back after receiving a storage event', () => {
+    initUserState('client-id')
+    localStorage.setItem(
+      userStateStorageKey,
+      JSON.stringify({
+        accessToken: 'access-token',
+        accessTokenExpiresAt: Date.now() + 60 * 60 * 1000,
+        refreshToken: 'refresh-token',
+        username: 'alice'
+      })
+    )
+    const setItem = vi.spyOn(Storage.prototype, 'setItem')
+
+    window.dispatchEvent(new StorageEvent('storage', { key: userStateStorageKey }))
+
+    expect(setItem).not.toHaveBeenCalled()
+  })
+})

spx-gui/src/stores/user/signed-in.ts

@@ -1,4 +1,4 @@
-import { reactive, watchEffect, computed } from 'vue'
+import { reactive, computed } from 'vue'
 import { composeQuery, useQuery, useQueryCache, useQueryWithCache } from '@/utils/query'
 import { capture, useAction } from '@/utils/exception'
 import { OAuthFlow, type OAuthTokenResponse } from '@/utils/oauth'
@@ -10,15 +10,25 @@ import { getUserQueryKey } from './query-keys'
 export type SignedInUser = userApis.SignedInUser
 
 const userStateStorageKey = 'builder-user'
+const userAccessTokenLockName = 'builder-user-access-token'
+
+type UserState = {
+  accessToken: string | null
+  accessTokenExpiresAt: number | null
+  refreshToken: string | null
+  username: string | null
+}
+
+const emptyUserState: UserState = {
+  accessToken: null,
+  accessTokenExpiresAt: null,
+  refreshToken: null,
+  username: null
+}
 
 let oauthFlow: OAuthFlow<{ returnTo: string }> | null = null
 
-const userState = reactive({
-  accessToken: null as string | null,
-  accessTokenExpiresAt: null as number | null,
-  refreshToken: null as string | null,
-  username: null as string | null
-})
+const userState = reactive<UserState>({ ...emptyUserState })
 
 function ensureOAuthFlow() {
   if (oauthFlow == null) throw new Error('OAuth flow is not initialized')
@@ -31,15 +41,26 @@ export function initUserState(clientId: string) {
     redirectUri: `${window.location.origin}/sign-in/callback`
   })
 
+  restoreUserState()
+  window.addEventListener('storage', (event) => {
+    if (event.key == null || event.key === userStateStorageKey) restoreUserState()
+  })
+}
+
+function restoreUserState() {
   const stored = localStorage.getItem(userStateStorageKey)
-  if (stored != null) {
-    try {
-      Object.assign(userState, JSON.parse(stored))
-    } catch {
-      localStorage.removeItem(userStateStorageKey)
-    }
+  try {
+    const newState: UserState = stored != null ? JSON.parse(stored) : emptyUserState
+    Object.assign(userState, newState)
+  } catch {
+    localStorage.removeItem(userStateStorageKey)
+    Object.assign(userState, emptyUserState)
   }
-  watchEffect(() => localStorage.setItem(userStateStorageKey, JSON.stringify(userState)))
+}
+
+function setUserState(state: UserState) {
+  Object.assign(userState, state)
+  localStorage.setItem(userStateStorageKey, JSON.stringify(state))
 }
 
 async function getSignedInUsernameByAccessToken(accessToken: string) {
@@ -49,10 +70,12 @@ async function getSignedInUsernameByAccessToken(accessToken: string) {
 
 async function handleTokenResponse(resp: OAuthTokenResponse) {
   const username = await getSignedInUsernameByAccessToken(resp.access_token)
-  userState.accessToken = resp.access_token
-  userState.accessTokenExpiresAt = resp.expires_in != null ? Date.now() + resp.expires_in * 1000 : null
-  userState.refreshToken = resp.refresh_token ?? null
-  userState.username = username
+  setUserState({
+    accessToken: resp.access_token,
+    accessTokenExpiresAt: resp.expires_in != null ? Date.now() + resp.expires_in * 1000 : null,
+    refreshToken: resp.refresh_token ?? null,
+    username
+  })
 }
 
 export function useSignIn() {
@@ -74,17 +97,16 @@ export async function completeSignIn(search: string) {
 
 export async function signInWithAccessToken(accessToken: string) {
   const username = await getSignedInUsernameByAccessToken(accessToken)
-  userState.accessToken = accessToken
-  userState.accessTokenExpiresAt = null
-  userState.refreshToken = null
-  userState.username = username
+  setUserState({
+    accessToken,
+    accessTokenExpiresAt: null,
+    refreshToken: null,
+    username
+  })
 }
 
 function clearUserState() {
-  userState.accessToken = null
-  userState.accessTokenExpiresAt = null
-  userState.refreshToken = null
-  userState.username = null
+  setUserState(emptyUserState)
 }
 
 export async function signOut() {
@@ -95,27 +117,27 @@ export async function signOut() {
   ).catch((e) => capture(e, 'Failed to revoke tokens during sign out'))
 }
 
-let tokenRefreshPromise: Promise<void> | null = null
-
 export async function ensureAccessToken(): Promise<string | null> {
   if (isAccessTokenValid()) return userState.accessToken
-  if (userState.refreshToken == null) {
-    clearUserState()
-    return null
-  }
-  if (tokenRefreshPromise == null) {
-    tokenRefreshPromise = ensureOAuthFlow()
-      .refreshToken(userState.refreshToken)
-      .then(handleTokenResponse)
-      .catch((e) => {
-        capture(e, 'Failed to refresh access token')
-        clearUserState()
-      })
-      .finally(() => {
-        tokenRefreshPromise = null
-      })
-  }
-  await tokenRefreshPromise
+
+  await navigator.locks.request(userAccessTokenLockName, async () => {
+    // Another tab may have refreshed the token while this request waited for the lock.
+    restoreUserState()
+    if (isAccessTokenValid()) return
+    if (userState.refreshToken == null) {
+      clearUserState()
+      return
+    }
+
+    const refreshTokenBeforeRefresh = userState.refreshToken
+    try {
+      const token = await ensureOAuthFlow().refreshToken(refreshTokenBeforeRefresh)
+      await handleTokenResponse(token)
+    } catch (e) {
+      capture(e, 'Failed to refresh access token')
+      clearUserState()
+    }
+  })
   return userState.accessToken
 }