From 11a8c1d2d1c83c3f61235af2b2aec8c2b6484d5f Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Fri, 17 Jan 2025 12:19:13 +0100 Subject: [PATCH] feat: improved insecure `getSession()` warning --- src/GoTrueClient.ts | 19 +++-------- src/lib/helpers.ts | 76 ++++++++++++++++++++++++++++++++++++++++- test/helpers.test.ts | 81 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 159 insertions(+), 17 deletions(-) diff --git a/src/GoTrueClient.ts b/src/GoTrueClient.ts index 0b7d5d0d2..e39b79b49 100644 --- a/src/GoTrueClient.ts +++ b/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,9 @@ 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( + 'User object comes from insecure storage and may not be authentic. Call getUser() instead to prevent security issues.' + ) } return { data: { session: currentSession }, error: null } diff --git a/src/lib/helpers.ts b/src/lib/helpers.ts index dee69fea9..f063b1fe7 100644 --- a/src/lib/helpers.ts +++ b/src/lib/helpers.ts @@ -1,5 +1,5 @@ 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) @@ -7,6 +7,20 @@ export function expiresAt(expiresIn: number) { } 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,63 @@ export function parseResponseAPIVersion(response: Response) { return null } } + +const warningSymbol = Symbol('WARNING') + +class UnavailableObject { + constructor(reason: string) { + ;(this as any)[warningSymbol] = reason + } + + toString() { + return JSON.stringify({ WARNING: (this as any)[warningSymbol] }) + } +} + +const PROPERTY_WARNINGS_SHOWN: { [reason: string]: boolean } = {} + +export function userNotAvailableProxy(reason: string): User { + const target = new UnavailableObject( + `@supabase/auth-js: Attempting to access a user object which is not available. Reason: ${reason}` + ) + + return new Proxy(target as User, { + get: (target: any, prop: PropertyKey, receiver: any) => { + if (typeof prop !== 'string' || prop in target) { + return Reflect.get(target, prop, receiver) + } + + let returnValue: any = undefined + switch (prop) { + case 'id': + case 'created_at': + case 'aud': + returnValue = '@@@ not-available @@@' + break + + case 'app_metadata': + case 'user_metadata': + returnValue = new UnavailableObject( + `@supabase/auth-js: User property object "${prop}" is not available. Reason: ${reason}` + ) + break + } + + if (!PROPERTY_WARNINGS_SHOWN[reason]) { + PROPERTY_WARNINGS_SHOWN[reason] = true + + console.warn( + `@supabase/auth-js: Accessing the "${prop}" (or any other) property of the user object under session is not supported, returned value will be ${ + returnValue === undefined + ? 'undefined' + : returnValue instanceof UnavailableObject + ? 'empty object' + : JSON.stringify(returnValue) + }. Reason: ${reason}` + ) + } + + return returnValue + }, + }) +} diff --git a/test/helpers.test.ts b/test/helpers.test.ts index 4cc0cfc83..f58fb0376 100644 --- a/test/helpers.test.ts +++ b/test/helpers.test.ts @@ -1,4 +1,9 @@ -import { parseParametersFromURL, parseResponseAPIVersion } from '../src/lib/helpers' +import { + parseParametersFromURL, + parseResponseAPIVersion, + userNotAvailableProxy, + uuid, +} from '../src/lib/helpers' describe('parseParametersFromURL', () => { it('should parse parameters from a URL with query params only', () => { @@ -71,3 +76,77 @@ describe('parseResponseAPIVersion', () => { }) }) }) + +describe('uuid', () => { + 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 + } + }) +}) + +describe('userNotAvailableProxy', () => { + it('should show a warning when calling toString()', () => { + expect(`${userNotAvailableProxy('REASON-0')}`).toMatchInlineSnapshot( + `"{\\"WARNING\\":\\"@supabase/auth-js: Attempting to access a user object which is not available. Reason: REASON-0\\"}"` + ) + }) + + 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 under session is not supported, returned value will be \\"@@@ not-available @@@\\". Reason: REASON-1", + ] + `) + numberOfWarnings += 1 + } + + const object = userNotAvailableProxy('REASON-1') + expect(object.id).toMatchInlineSnapshot(`"@@@ not-available @@@"`) + expect(object.created_at).toMatchInlineSnapshot(`"@@@ not-available @@@"`) + expect(object.aud).toMatchInlineSnapshot(`"@@@ not-available @@@"`) + expect(object.app_metadata).toMatchInlineSnapshot(` + UnavailableObject { + Symbol(WARNING): "@supabase/auth-js: User property object \\"app_metadata\\" is not available. Reason: REASON-1", + } + `) + expect(object.user_metadata).toMatchInlineSnapshot(` + UnavailableObject { + Symbol(WARNING): "@supabase/auth-js: User property object \\"user_metadata\\" is not available. Reason: REASON-1", + } + `) + expect(object.email).toMatchInlineSnapshot(`undefined`) + + expect(numberOfWarnings).toEqual(1) + } finally { + console.warn = originalWarn + } + }) +})