diff --git a/spx-gui/src/stores/user/signed-in.test.ts b/spx-gui/src/stores/user/signed-in.test.ts new file mode 100644 index 000000000..6f36f4cd5 --- /dev/null +++ b/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) => { + 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) => 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) => 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) => 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() + }) +}) diff --git a/spx-gui/src/stores/user/signed-in.ts b/spx-gui/src/stores/user/signed-in.ts index 78f06cf3c..be68698c4 100644 --- a/spx-gui/src/stores/user/signed-in.ts +++ b/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({ ...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 | null = null - export async function ensureAccessToken(): Promise { 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 }