diff --git a/.github/workflows/dogfooding.yml b/.github/workflows/dogfooding.yml index 03188b75..24f5a753 100644 --- a/.github/workflows/dogfooding.yml +++ b/.github/workflows/dogfooding.yml @@ -4,11 +4,12 @@ on: pull_request_review: types: [submitted, edited] - pull_request_target: - types: - - opened - branches: - - '*' + pull_request: + + push: + +permissions: + contents: read jobs: check_dogfooding: diff --git a/src/GoTrueClient.ts b/src/GoTrueClient.ts index 609a9020..002ebe53 100644 --- a/src/GoTrueClient.ts +++ b/src/GoTrueClient.ts @@ -41,14 +41,15 @@ import { uuid, retryable, sleep, - supportsLocalStorage, parseParametersFromURL, getCodeChallengeAndMethod, getAlgorithm, validateExp, decodeJWT, + userNotAvailableProxy, + supportsLocalStorage, } from './lib/helpers' -import { localStorageAdapter, memoryLocalStorageAdapter } from './lib/local-storage' +import { memoryLocalStorageAdapter } from './lib/local-storage' import { polyfillGlobalThis } from './lib/polyfills' import { version } from './lib/version' import { LockAcquireTimeoutError, navigatorLock } from './lib/locks' @@ -114,7 +115,10 @@ import { stringToUint8Array, bytesToBase64URL } from './lib/base64url' polyfillGlobalThis() // Make "globalThis" available -const DEFAULT_OPTIONS: Omit, 'fetch' | 'storage' | 'lock'> = { +const DEFAULT_OPTIONS: Omit< + Required, + 'fetch' | 'storage' | 'userStorage' | 'lock' +> = { url: GOTRUE_URL, storageKey: STORAGE_KEY, autoRefreshToken: true, @@ -158,6 +162,10 @@ export default class GoTrueClient { protected autoRefreshToken: boolean protected persistSession: boolean protected storage: SupportedStorage + /** + * @experimental + */ + protected userStorage: SupportedStorage | null = null protected memoryStorage: { [key: string]: string } | null = null protected stateChangeEmitters: Map = new Map() protected autoRefreshTicker: ReturnType | null = null @@ -251,12 +259,16 @@ export default class GoTrueClient { this.storage = settings.storage } else { if (supportsLocalStorage()) { - this.storage = localStorageAdapter + this.storage = globalThis.localStorage } else { this.memoryStorage = {} this.storage = memoryLocalStorageAdapter(this.memoryStorage) } } + + if (settings.userStorage) { + this.userStorage = settings.userStorage + } } else { this.memoryStorage = {} this.storage = memoryLocalStorageAdapter(this.memoryStorage) @@ -1347,7 +1359,20 @@ export default class GoTrueClient { ) if (!hasExpired) { - if (this.storage.isServer) { + if (this.userStorage) { + const maybeUser: { user?: User | null } | null = (await getItemAsync( + this.userStorage, + this.storageKey + '-user' + )) as any + + if (maybeUser?.user) { + currentSession.user = maybeUser.user + } else { + currentSession.user = userNotAvailableProxy() + } + } + + if (this.storage.isServer && currentSession.user) { let suppressWarning = this.suppressGetSessionWarning const proxySession: Session = new Proxy(currentSession, { get: (target: any, prop: string, receiver: any) => { @@ -2128,7 +2153,47 @@ export default class GoTrueClient { this._debug(debugName, 'begin') try { - const currentSession = await getItemAsync(this.storage, this.storageKey) + const currentSession: Session = (await getItemAsync(this.storage, this.storageKey)) as any + + if (this.userStorage) { + let maybeUser: { user: User | null } | null = (await getItemAsync( + this.userStorage, + this.storageKey + '-user' + )) as any + + if (!this.storage.isServer && Object.is(this.storage, this.userStorage) && !maybeUser) { + // storage and userStorage are the same storage medium, for example + // window.localStorage if userStorage does not have the user from + // storage stored, store it first thereby migrating the user object + // from storage -> userStorage + + maybeUser = { user: currentSession.user } + await setItemAsync(this.userStorage, this.storageKey + '-user', maybeUser) + } + + currentSession.user = maybeUser?.user ?? userNotAvailableProxy() + } else if (currentSession && !currentSession.user) { + // user storage is not set, let's check if it was previously enabled so + // we bring back the storage as it should be + + if (!currentSession.user) { + // test if userStorage was previously enabled and the storage medium was the same, to move the user back under the same key + const separateUser: { user: User | null } | null = (await getItemAsync( + this.storage, + this.storageKey + '-user' + )) as any + + if (separateUser && separateUser?.user) { + currentSession.user = separateUser.user + + await removeItemAsync(this.storage, this.storageKey + '-user') + await setItemAsync(this.storage, this.storageKey, currentSession) + } else { + currentSession.user = userNotAvailableProxy() + } + } + } + this._debug(debugName, 'session from storage', currentSession) if (!this._isValidSession(currentSession)) { @@ -2165,6 +2230,29 @@ export default class GoTrueClient { } } } + } else if ( + currentSession.user && + (currentSession.user as any).__isUserNotAvailableProxy === true + ) { + // If we have a proxy user, try to get the real user data + try { + const { data, error: userError } = await this._getUser(currentSession.access_token) + + if (!userError && data?.user) { + currentSession.user = data.user + await this._saveSession(currentSession) + await this._notifyAllSubscribers('SIGNED_IN', currentSession) + } else { + this._debug(debugName, 'could not get user data, skipping SIGNED_IN notification') + } + } catch (getUserError) { + console.error('Error getting user data:', getUserError) + this._debug( + debugName, + 'error getting user data, skipping SIGNED_IN notification', + getUserError + ) + } } else { // no need to persist currentSession again, as we just loaded it from // local storage; persisting it again may overwrite a value saved by @@ -2278,13 +2366,52 @@ export default class GoTrueClient { // _saveSession is always called whenever a new session has been acquired // so we can safely suppress the warning returned by future getSession calls this.suppressGetSessionWarning = true - await setItemAsync(this.storage, this.storageKey, session) + + // Create a shallow copy to work with, to avoid mutating the original session object if it's used elsewhere + const sessionToProcess = { ...session } + + const userIsProxy = + sessionToProcess.user && (sessionToProcess.user as any).__isUserNotAvailableProxy === true + if (this.userStorage) { + if (!userIsProxy && sessionToProcess.user) { + // If it's a real user object, save it to userStorage. + await setItemAsync(this.userStorage, this.storageKey + '-user', { + user: sessionToProcess.user, + }) + } else if (userIsProxy) { + // If it's the proxy, it means user was not found in userStorage. + // We should ensure no stale user data for this key exists in userStorage if we were to save null, + // or simply not save the proxy. For now, we don't save the proxy here. + // If there's a need to clear userStorage if user becomes proxy, that logic would go here. + } + + // Prepare the main session data for primary storage: remove the user property before cloning + // This is important because the original session.user might be the proxy + const mainSessionData: Omit & { user?: User } = { ...sessionToProcess } + delete mainSessionData.user // Remove user (real or proxy) before cloning for main storage + + const clonedMainSessionData = structuredClone(mainSessionData) + await setItemAsync(this.storage, this.storageKey, clonedMainSessionData) + } else { + // No userStorage is configured. + // In this case, session.user should ideally not be a proxy. + // If it were, structuredClone would fail. This implies an issue elsewhere if user is a proxy here + const clonedSession = structuredClone(sessionToProcess) // sessionToProcess still has its original user property + await setItemAsync(this.storage, this.storageKey, clonedSession) + } } private async _removeSession() { this._debug('#_removeSession()') await removeItemAsync(this.storage, this.storageKey) + await removeItemAsync(this.storage, this.storageKey + '-code-verifier') + await removeItemAsync(this.storage, this.storageKey + '-user') + + if (this.userStorage) { + await removeItemAsync(this.userStorage, this.storageKey + '-user') + } + await this._notifyAllSubscribers('SIGNED_OUT', null) } diff --git a/src/lib/helpers.ts b/src/lib/helpers.ts index 6931423b..6dae3511 100644 --- a/src/lib/helpers.ts +++ b/src/lib/helpers.ts @@ -1,7 +1,7 @@ import { API_VERSION_HEADER_NAME, BASE64URL_REGEX } from './constants' import { AuthInvalidJwtError } from './errors' import { base64UrlToUint8Array, stringFromBase64URL } from './base64url' -import { JwtHeader, JwtPayload, SupportedStorage } from './types' +import { JwtHeader, JwtPayload, SupportedStorage, User } from './types' export function expiresAt(expiresIn: number) { const timeNow = Math.round(Date.now() / 1000) @@ -365,3 +365,41 @@ export function validateUUID(str: string) { throw new Error('@supabase/auth-js: Expected parameter to be UUID but is not') } } + +export function userNotAvailableProxy(): User { + const proxyTarget = {} as User + + return new Proxy(proxyTarget, { + get: (target: any, prop: string) => { + if (prop === '__isUserNotAvailableProxy') { + return true + } + // Preventative check for common problematic symbols during cloning/inspection + // These symbols might be accessed by structuredClone or other internal mechanisms. + if (typeof prop === 'symbol') { + const sProp = (prop as symbol).toString() + if ( + sProp === 'Symbol(Symbol.toPrimitive)' || + sProp === 'Symbol(Symbol.toStringTag)' || + sProp === 'Symbol(util.inspect.custom)' + ) { + // Node.js util.inspect + return undefined + } + } + throw new Error( + `@supabase/auth-js: client was created with userStorage option and there was no user stored in the user storage. Accessing the "${prop}" property of the session object is not supported. Please use getUser() instead.` + ) + }, + set: (_target: any, prop: string) => { + throw new Error( + `@supabase/auth-js: client was created with userStorage option and there was no user stored in the user storage. Setting the "${prop}" property of the session object is not supported. Please use getUser() to fetch a user object you can manipulate.` + ) + }, + deleteProperty: (_target: any, prop: string) => { + throw new Error( + `@supabase/auth-js: client was created with userStorage option and there was no user stored in the user storage. Deleting the "${prop}" property of the session object is not supported. Please use getUser() to fetch a user object you can manipulate.` + ) + }, + }) +} diff --git a/src/lib/local-storage.ts b/src/lib/local-storage.ts index d103f564..0424b63c 100644 --- a/src/lib/local-storage.ts +++ b/src/lib/local-storage.ts @@ -1,33 +1,5 @@ -import { supportsLocalStorage } from './helpers' import { SupportedStorage } from './types' -/** - * Provides safe access to the globalThis.localStorage property. - */ -export const localStorageAdapter: SupportedStorage = { - getItem: (key) => { - if (!supportsLocalStorage()) { - return null - } - - return globalThis.localStorage.getItem(key) - }, - setItem: (key, value) => { - if (!supportsLocalStorage()) { - return - } - - globalThis.localStorage.setItem(key, value) - }, - removeItem: (key) => { - if (!supportsLocalStorage()) { - return - } - - globalThis.localStorage.removeItem(key) - }, -} - /** * Returns a localStorage-like object that stores the key-value pairs in * memory. diff --git a/src/lib/types.ts b/src/lib/types.ts index 7b5f83ea..43798586 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -70,6 +70,14 @@ export type GoTrueClientOptions = { persistSession?: boolean /* Provide your own local storage implementation to use instead of the browser's local storage. */ storage?: SupportedStorage + /** + * Stores the user object in a separate storage location from the rest of the session data. When non-null, `storage` will only store a JSON object containing the access and refresh token and some adjacent metadata, while `userStorage` will only contain the user object under the key `storageKey + '-user'`. + * + * When this option is set and cookie storage is used, `getSession()` and other functions that load a session from the cookie store might not return back a user. It's very important to always use `getUser()` to fetch a user object in those scenarios. + * + * @experimental + */ + userStorage?: SupportedStorage /* A custom fetch implementation. */ fetch?: Fetch /* If set to 'pkce' PKCE flow. Defaults to the 'implicit' flow otherwise */ @@ -253,6 +261,10 @@ export interface Session { */ expires_at?: number token_type: string + + /** + * When using a separate user storage, accessing properties of this object will throw an error. + */ user: User } diff --git a/test/lib/local-storage.test.ts b/test/lib/local-storage.test.ts index ae51cfcf..ddc301d2 100644 --- a/test/lib/local-storage.test.ts +++ b/test/lib/local-storage.test.ts @@ -1,5 +1,4 @@ -import { memoryLocalStorageAdapter, localStorageAdapter } from '../../src/lib/local-storage' -import * as helpers from '../../src/lib/helpers' +import { memoryLocalStorageAdapter } from '../../src/lib/local-storage' describe('memoryLocalStorageAdapter', () => { it('sets and gets a value', () => { @@ -20,74 +19,3 @@ describe('memoryLocalStorageAdapter', () => { expect(adapter.getItem('key')).toBeNull() }) }) - -describe('localStorageAdapter', () => { - const mockLocalStorage = { - getItem: jest.fn(() => 'value'), - setItem: jest.fn(), - removeItem: jest.fn(), - } - - beforeEach(() => { - jest.clearAllMocks() - Object.defineProperty(globalThis, 'localStorage', { - value: mockLocalStorage, - configurable: true, - }) - }) - - it('calls localStorage.getItem when supported', () => { - jest.spyOn(helpers, 'supportsLocalStorage').mockReturnValue(true) - expect(localStorageAdapter.getItem('key')).toBe('value') - expect(globalThis.localStorage.getItem).toHaveBeenCalledWith('key') - }) - - it('does nothing if localStorage is not supported', () => { - jest.spyOn(helpers, 'supportsLocalStorage').mockReturnValue(false) - expect(localStorageAdapter.getItem('key')).toBeNull() - }) - - describe('setItem', () => { - it('calls localStorage.setItem when supported', () => { - jest.spyOn(helpers, 'supportsLocalStorage').mockReturnValue(true) - const key = 'test-key' - const value = 'test-value' - - localStorageAdapter.setItem(key, value) - - expect(mockLocalStorage.setItem).toHaveBeenCalledWith(key, value) - expect(mockLocalStorage.setItem).toHaveBeenCalledTimes(1) - }) - - it('does nothing if localStorage is not supported', () => { - jest.spyOn(helpers, 'supportsLocalStorage').mockReturnValue(false) - const key = 'test-key' - const value = 'test-value' - - localStorageAdapter.setItem(key, value) - - expect(mockLocalStorage.setItem).not.toHaveBeenCalled() - }) - }) - - describe('removeItem', () => { - it('calls localStorage.removeItem when supported', () => { - jest.spyOn(helpers, 'supportsLocalStorage').mockReturnValue(true) - const key = 'test-key' - - localStorageAdapter.removeItem(key) - - expect(mockLocalStorage.removeItem).toHaveBeenCalledWith(key) - expect(mockLocalStorage.removeItem).toHaveBeenCalledTimes(1) - }) - - it('does nothing if localStorage is not supported', () => { - jest.spyOn(helpers, 'supportsLocalStorage').mockReturnValue(false) - const key = 'test-key' - - localStorageAdapter.removeItem(key) - - expect(mockLocalStorage.removeItem).not.toHaveBeenCalled() - }) - }) -})