fix(auth): only warn if multiple clients share a storage-key (#1767)

Author: sh41

packages/core/auth-js/src/GoTrueClient.ts

@@ -188,7 +188,7 @@ async function lockNoOp<R>(name: string, acquireTimeout: number, fn: () => Promi
 const GLOBAL_JWKS: { [storageKey: string]: { cachedAt: number; jwks: { keys: JWK[] } } } = {}
 
 export default class GoTrueClient {
-  private static nextInstanceID = 0
+  private static nextInstanceID: Record<string, number> = {}
 
   private instanceID: number
 
@@ -277,24 +277,26 @@ export default class GoTrueClient {
    * Create a new client for use in the browser.
    */
   constructor(options: GoTrueClientOptions) {
-    this.instanceID = GoTrueClient.nextInstanceID
-    GoTrueClient.nextInstanceID += 1
-
-    if (this.instanceID > 0 && isBrowser()) {
-      console.warn(
-        'Multiple GoTrueClient instances detected in the same browser context. It is not an error, but this should be avoided as it may produce undefined behavior when used concurrently under the same storage key.'
-      )
-    }
-
     const settings = { ...DEFAULT_OPTIONS, ...options }
+    this.storageKey = settings.storageKey
+
+    this.instanceID = GoTrueClient.nextInstanceID[this.storageKey] ?? 0
+    GoTrueClient.nextInstanceID[this.storageKey] = this.instanceID + 1
 
     this.logDebugMessages = !!settings.debug
     if (typeof settings.debug === 'function') {
       this.logger = settings.debug
     }
 
+    if (this.instanceID > 0 && isBrowser()) {
+      const message = `${this._logPrefix()} Multiple GoTrueClient instances detected in the same browser context. It is not an error, but this should be avoided as it may produce undefined behavior when used concurrently under the same storage key.`
+      console.warn(message)
+      if (this.logDebugMessages) {
+        console.trace(message)
+      }
+    }
+
     this.persistSession = settings.persistSession
-    this.storageKey = settings.storageKey
     this.autoRefreshToken = settings.autoRefreshToken
     this.admin = new GoTrueAdminApi({
       url: settings.url,
@@ -400,12 +402,16 @@ export default class GoTrueClient {
     return result
   }
 
+  private _logPrefix(): string {
+    return (
+      'GoTrueClient@' +
+      `${this.storageKey}:${this.instanceID} (${version}) ${new Date().toISOString()}`
+    )
+  }
+
   private _debug(...args: any[]): GoTrueClient {
     if (this.logDebugMessages) {
-      this.logger(
-        `GoTrueClient@${this.instanceID} (${version}) ${new Date().toISOString()}`,
-        ...args
-      )
+      this.logger(this._logPrefix(), ...args)
     }
 
     return this

packages/core/auth-js/test/GoTrueClient.browser.test.ts

@@ -2,7 +2,12 @@
  * @jest-environment jsdom
  */
 
-import { autoRefreshClient, getClientWithSpecificStorage, pkceClient } from './lib/clients'
+import {
+  autoRefreshClient,
+  getClientWithSpecificStorage,
+  getClientWithSpecificStorageKey,
+  pkceClient,
+} from './lib/clients'
 import { mockUserCredentials } from './lib/utils'
 import {
   supportsLocalStorage,
@@ -190,6 +195,94 @@ describe('Fetch resolution in browser environment', () => {
     const resolvedFetch = resolveFetch(customFetch)
     expect(typeof resolvedFetch).toBe('function')
   })
+
+  it('should warn when two clients are created with the same storage key', () => {
+    let consoleWarnSpy
+    let consoleTraceSpy
+    try {
+      consoleWarnSpy = jest.spyOn(console, 'warn')
+      consoleTraceSpy = jest.spyOn(console, 'trace')
+      getClientWithSpecificStorageKey('same-storage-key')
+      getClientWithSpecificStorageKey('same-storage-key')
+      expect(consoleWarnSpy).toHaveBeenCalledTimes(1)
+      expect(consoleWarnSpy).toHaveBeenCalledWith(
+        expect.stringMatching(
+          /GoTrueClient@same-storage-key:1 .* Multiple GoTrueClient instances detected in the same browser context. It is not an error, but this should be avoided as it may produce undefined behavior when used concurrently under the same storage key./
+        )
+      )
+      expect(consoleTraceSpy).not.toHaveBeenCalled()
+    } finally {
+      consoleWarnSpy?.mockRestore()
+      consoleTraceSpy?.mockRestore()
+    }
+  })
+
+  it('should warn & trace when two clients are created with the same storage key and debug is enabled', () => {
+    let consoleWarnSpy
+    let consoleTraceSpy
+    try {
+      consoleWarnSpy = jest.spyOn(console, 'warn')
+      consoleTraceSpy = jest.spyOn(console, 'trace')
+      getClientWithSpecificStorageKey('identical-storage-key')
+      getClientWithSpecificStorageKey('identical-storage-key', { debug: true })
+      expect(consoleWarnSpy).toHaveBeenCalledTimes(1)
+      expect(consoleWarnSpy).toHaveBeenCalledWith(
+        expect.stringMatching(
+          /GoTrueClient@identical-storage-key:1 .* Multiple GoTrueClient instances detected in the same browser context. It is not an error, but this should be avoided as it may produce undefined behavior when used concurrently under the same storage key./
+        )
+      )
+      expect(consoleTraceSpy).toHaveBeenCalledWith(
+        expect.stringMatching(
+          /GoTrueClient@identical-storage-key:1 .* Multiple GoTrueClient instances detected in the same browser context. It is not an error, but this should be avoided as it may produce undefined behavior when used concurrently under the same storage key./
+        )
+      )
+    } finally {
+      consoleWarnSpy?.mockRestore()
+      consoleTraceSpy?.mockRestore()
+    }
+  })
+
+  it('should not warn when two clients are created with differing storage keys', () => {
+    let consoleWarnSpy
+    let consoleTraceSpy
+    try {
+      consoleWarnSpy = jest.spyOn(console, 'warn')
+      consoleTraceSpy = jest.spyOn(console, 'trace')
+      getClientWithSpecificStorageKey('first-storage-key')
+      getClientWithSpecificStorageKey('second-storage-key')
+      expect(consoleWarnSpy).not.toHaveBeenCalled()
+      expect(consoleTraceSpy).not.toHaveBeenCalled()
+    } finally {
+      consoleWarnSpy?.mockRestore()
+      consoleTraceSpy?.mockRestore()
+    }
+  })
+
+  it('should warn only when a second client with a duplicate key is created', () => {
+    let consoleWarnSpy
+    let consoleTraceSpy
+    try {
+      consoleWarnSpy = jest.spyOn(console, 'warn')
+      consoleTraceSpy = jest.spyOn(console, 'trace')
+      getClientWithSpecificStorageKey('test-storage-key1')
+      expect(consoleWarnSpy).not.toHaveBeenCalled()
+      getClientWithSpecificStorageKey('test-storage-key2')
+      expect(consoleWarnSpy).not.toHaveBeenCalled()
+      getClientWithSpecificStorageKey('test-storage-key3')
+      expect(consoleWarnSpy).not.toHaveBeenCalled()
+      getClientWithSpecificStorageKey('test-storage-key2')
+      expect(consoleWarnSpy).toHaveBeenCalledTimes(1)
+      expect(consoleWarnSpy).toHaveBeenCalledWith(
+        expect.stringMatching(
+          /GoTrueClient@test-storage-key2:1 .* Multiple GoTrueClient instances detected in the same browser context. It is not an error, but this should be avoided as it may produce undefined behavior when used concurrently under the same storage key./
+        )
+      )
+      expect(consoleTraceSpy).not.toHaveBeenCalled()
+    } finally {
+      consoleWarnSpy?.mockRestore()
+      consoleTraceSpy?.mockRestore()
+    }
+  })
 })
 
 describe('Callback URL handling', () => {

packages/core/auth-js/test/lib/clients.ts

@@ -1,5 +1,5 @@
 import jwt from 'jsonwebtoken'
-import { GoTrueAdminApi, GoTrueClient } from '../../src/index'
+import { GoTrueAdminApi, GoTrueClient, type GoTrueClientOptions } from '../../src/index'
 import { SupportedStorage } from '../../src/lib/types'
 
 export const SIGNUP_ENABLED_AUTO_CONFIRM_OFF_PORT = 9999
@@ -156,3 +156,16 @@ export function getClientWithSpecificStorage(storage: SupportedStorage) {
     storage,
   })
 }
+
+export function getClientWithSpecificStorageKey(
+  storageKey: string,
+  opts: GoTrueClientOptions = {}
+) {
+  return new GoTrueClient({
+    url: GOTRUE_URL_SIGNUP_ENABLED_AUTO_CONFIRM_ON,
+    autoRefreshToken: false,
+    persistSession: true,
+    storageKey,
+    ...opts,
+  })
+}