feat: improved insecure getSession() warning

Author: hf

src/GoTrueClient.ts

@@ -36,6 +36,7 @@ import {
   supportsLocalStorage,
   parseParametersFromURL,
   getCodeChallengeAndMethod,
+  userNotAvailableProxy,
 } from './lib/helpers'
 import { localStorageAdapter, memoryLocalStorageAdapter } from './lib/local-storage'
 import { polyfillGlobalThis } from './lib/polyfills'
@@ -1120,21 +1121,10 @@ export default class GoTrueClient {
 
       if (!hasExpired) {
         if (this.storage.isServer) {
-          let suppressWarning = this.suppressGetSessionWarning
-          const proxySession: Session = new Proxy(currentSession, {
-            get: (target: any, prop: string, receiver: any) => {
-              if (!suppressWarning && prop === 'user') {
-                // only show warning when the user object is being accessed from the server
-                console.warn(
-                  'Using the user object as returned from supabase.auth.getSession() or from some supabase.auth.onAuthStateChange() events could be insecure! This value comes directly from the storage medium (usually cookies on the server) and may not be authentic. Use supabase.auth.getUser() instead which authenticates the data by contacting the Supabase Auth server.'
-                )
-                suppressWarning = true // keeps this proxy instance from logging additional warnings
-                this.suppressGetSessionWarning = true // keeps this client's future proxy instances from warning
-              }
-              return Reflect.get(target, prop, receiver)
-            },
-          })
-          currentSession = proxySession
+          currentSession.user = userNotAvailableProxy(
+            currentSession.user,
+            'User object comes from insecure storage and may not be authentic. Call getUser() instead to prevent security issues.'
+          )
         }
 
         return { data: { session: currentSession }, error: null }

src/lib/helpers.ts

@@ -1,12 +1,26 @@
 import { API_VERSION_HEADER_NAME } from './constants'
-import { SupportedStorage } from './types'
+import { SupportedStorage, User } from './types'
 
 export function expiresAt(expiresIn: number) {
   const timeNow = Math.round(Date.now() / 1000)
   return timeNow + expiresIn
 }
 
 export function uuid() {
+  if (
+    'crypto' in globalThis &&
+    typeof globalThis.crypto === 'object' &&
+    'randomUUID' in globalThis.crypto &&
+    typeof globalThis.crypto.randomUUID === 'function'
+  ) {
+    try {
+      return globalThis.crypto.randomUUID()
+    } catch (e: any) {
+      // ignore and fall back to below implementation, as crypto.randomUUID()
+      // only works in secure contexts
+    }
+  }
+
   return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function (c) {
     const r = (Math.random() * 16) | 0,
       v = c == 'x' ? r : (r & 0x3) | 0x8
@@ -344,3 +358,52 @@ export function parseResponseAPIVersion(response: Response) {
     return null
   }
 }
+
+function hasOwnProperty(target: any, property: any) {
+  if (!(target instanceof Object) || target === null) {
+    return false
+  }
+
+  if ('hasOwn' in Object && typeof (Object as any).hasOwn === 'function') {
+    return (Object as any).hasOwn(target, property)
+  }
+
+  return Object.prototype.hasOwnProperty.call(target, property)
+}
+
+const WARNING_SYMBOL = Symbol('WARNING')
+const PROPERTY_WARNINGS_SHOWN: { [reason: string]: boolean } = {}
+
+export function userNotAvailableProxy(target: User, reason: string): User {
+  const warning = `@supabase/auth-js: Accessing any property of this object is insecure. Reason: ${reason}` // this shows the warning in console.log, Inspector or other object dumps
+
+  ;(target as any)[WARNING_SYMBOL] = warning
+
+  return new Proxy(target, {
+    get: (target: User, prop: PropertyKey, receiver: any) => {
+      if (prop === 'toString') {
+        return () => `WARNING: ${warning} -- ${Reflect.get(target, prop, receiver).call(target)}`
+      }
+
+      if (
+        !PROPERTY_WARNINGS_SHOWN[reason] &&
+        typeof prop === 'string' &&
+        hasOwnProperty(target, prop)
+      ) {
+        PROPERTY_WARNINGS_SHOWN[reason] = true
+
+        console.warn(
+          `@supabase/auth-js: Accessing the "${prop}" (or any other) property of the user object is not secure. Reason: ${reason}`
+        )
+      }
+
+      const value = Reflect.get(target, prop, receiver)
+
+      if (typeof value === 'object' && value && !value[WARNING_SYMBOL]) {
+        value[WARNING_SYMBOL] = warning
+      }
+
+      return value
+    },
+  })
+}

test/GoTrueClient.test.ts

@@ -958,7 +958,7 @@ describe('GoTrueClient with storageisServer = true', () => {
     expect(warnings.length).toEqual(0)
   })
 
-  test('getSession() emits insecure warning, once per server client, when user object is accessed', async () => {
+  test('getSession() emits insecure warning, once per server client, when property on user object is accessed', async () => {
     const storage = memoryLocalStorageAdapter({
       [STORAGE_KEY]: JSON.stringify({
         access_token: 'jwt.accesstoken.signature',
@@ -981,29 +981,26 @@ describe('GoTrueClient with storageisServer = true', () => {
       data: { session },
     } = await client.getSession()
 
-    const user = session?.user // accessing the user object from getSession should emit a warning the first time
-    expect(user).not.toBeNull()
-    expect(warnings.length).toEqual(1)
-    expect(
-      warnings[0][0].startsWith(
-        'Using the user object as returned from supabase.auth.getSession() '
-      )
-    ).toEqual(true)
-
-    const user2 = session?.user // accessing the user object further should not emit a warning
-    expect(user2).not.toBeNull()
-    expect(warnings.length).toEqual(1)
-
-    const {
-      data: { session: session2 },
-    } = await client.getSession() // create new proxy instance
-
-    const user3 = session2?.user // accessing the user object in subsequent proxy instances, for this client, should not emit a warning
-    expect(user3).not.toBeNull()
-    expect(warnings.length).toEqual(1)
+    expect(session?.user).not.toBeNull()
+    expect(session?.user?.id).toMatchInlineSnapshot(`"random-user-id"`)
+    expect(warnings).toMatchInlineSnapshot(`
+      Array [
+        Array [
+          "@supabase/auth-js: Accessing the \\"id\\" (or any other) property of the user object is not secure. Reason: User object comes from insecure storage and may not be authentic. Call getUser() instead to prevent security issues.",
+        ],
+      ]
+    `)
+    expect(session?.user?.app_metadata).not.toBeNull()
+    expect(warnings).toMatchInlineSnapshot(`
+      Array [
+        Array [
+          "@supabase/auth-js: Accessing the \\"id\\" (or any other) property of the user object is not secure. Reason: User object comes from insecure storage and may not be authentic. Call getUser() instead to prevent security issues.",
+        ],
+      ]
+    `)
   })
 
-  test('getSession emits no warnings if getUser is called prior', async () => {
+  test('getSession() emits no warnings if getUser() is called prior', async () => {
     const client = new GoTrueClient({
       url: GOTRUE_URL_SIGNUP_ENABLED_AUTO_CONFIRM_ON,
       autoRefreshToken: false,
@@ -1021,15 +1018,14 @@ describe('GoTrueClient with storageisServer = true', () => {
       error,
     } = await client.getUser() // should suppress any warnings
     expect(error).toBeNull()
-    expect(user).not.toBeNull()
 
     const {
       data: { session },
     } = await client.getSession()
 
-    const sessionUser = session?.user // accessing the user object from getSession shouldn't emit a warning
-    expect(sessionUser).not.toBeNull()
-    expect(warnings.length).toEqual(0)
+    // accessing the user object from getSession shouldn't emit a warning
+    expect(session?.user?.id).not.toBeNull()
+    expect(warnings).toMatchInlineSnapshot(`Array []`)
   })
 
   test('saveSession should overwrite the existing session', async () => {

test/helpers.test.ts

@@ -1,4 +1,10 @@
-import { parseParametersFromURL, parseResponseAPIVersion } from '../src/lib/helpers'
+import { User } from '../src/lib/types'
+import {
+  parseParametersFromURL,
+  parseResponseAPIVersion,
+  userNotAvailableProxy,
+  uuid,
+} from '../src/lib/helpers'
 
 describe('parseParametersFromURL', () => {
   it('should parse parameters from a URL with query params only', () => {
@@ -71,3 +77,99 @@ describe('parseResponseAPIVersion', () => {
     })
   })
 })
+
+describe('uuid', () => {
+  if ('crypto' in globalThis) {
+    // nodejs 18, 20 don't have crypto
+
+    it('should generate a uuid when crypto.randomUUID() throws an error', () => {
+      const originalRandomUUID = crypto.randomUUID
+
+      try {
+        crypto.randomUUID = () => {
+          throw new Error('Fail for test')
+        }
+
+        expect(typeof uuid()).toEqual('string')
+      } finally {
+        crypto.randomUUID = originalRandomUUID
+      }
+    })
+
+    it('should generate a uuid with crypto.randomUUID()', () => {
+      const originalRandomUUID = crypto.randomUUID
+
+      try {
+        crypto.randomUUID = () => {
+          return 'random-uuid'
+        }
+
+        expect(uuid()).toEqual('random-uuid')
+      } finally {
+        crypto.randomUUID = originalRandomUUID
+      }
+    })
+  }
+
+  it('should generate a uuid', () => {
+    expect(typeof uuid()).toEqual('string')
+  })
+})
+
+describe('userNotAvailableProxy', () => {
+  it('should show a warning when calling toString()', () => {
+    expect(`${userNotAvailableProxy({} as User, 'REASON-0')}`).toMatchInlineSnapshot(
+      `"WARNING: @supabase/auth-js: Accessing any property of this object is insecure. Reason: REASON-0 -- [object Object]"`
+    )
+  })
+
+  it('should show a warning when calling toString()', () => {
+    const originalWarn = console.warn
+
+    try {
+      let numberOfWarnings = 0
+      console.warn = (...args: any[]) => {
+        expect(args).toMatchInlineSnapshot(`
+          Array [
+            "@supabase/auth-js: Accessing the \\"id\\" (or any other) property of the user object is not secure. Reason: REASON-1",
+          ]
+        `)
+        numberOfWarnings += 1
+      }
+
+      const object = userNotAvailableProxy(
+        {
+          id: 'user-id',
+          created_at: new Date(0).toISOString(),
+          aud: 'audience',
+          app_metadata: {},
+          user_metadata: {},
+        },
+        'REASON-1'
+      )
+
+      expect(`${object}`).toMatchInlineSnapshot(
+        `"WARNING: @supabase/auth-js: Accessing any property of this object is insecure. Reason: REASON-1 -- [object Object]"`
+      )
+
+      expect(object.id).toMatchInlineSnapshot(`"user-id"`)
+      expect(object.created_at).toMatchInlineSnapshot(`"1970-01-01T00:00:00.000Z"`)
+      expect(object.aud).toMatchInlineSnapshot(`"audience"`)
+      expect(object.app_metadata).toMatchInlineSnapshot(`
+        Object {
+          Symbol(WARNING): "@supabase/auth-js: Accessing any property of this object is insecure. Reason: REASON-1",
+        }
+      `)
+      expect(object.user_metadata).toMatchInlineSnapshot(`
+        Object {
+          Symbol(WARNING): "@supabase/auth-js: Accessing any property of this object is insecure. Reason: REASON-1",
+        }
+      `)
+      expect(object.email).toMatchInlineSnapshot(`undefined`)
+
+      expect(numberOfWarnings).toEqual(1)
+    } finally {
+      console.warn = originalWarn
+    }
+  })
+})