From 4ecfdda65188b71322753e57622be8eafe97ed6b Mon Sep 17 00:00:00 2001 From: Jason Creviston Date: Thu, 9 May 2024 05:16:40 -0400 Subject: [PATCH] fix: limit proxy session warning to once per client instance (#900) ## What kind of change does this PR introduce? Bug fix. ## What is the current behavior? A call to `getSession()`, when using server storage, logs a warning every time `session.user` is accessed. This is causing a lot of people to see multiple consecutive logs to the console - especially for SvelteKit users. ## What is the new behavior? Ensures that accessing `session.user`, from a `getSession()` call, only logs the warning once per Proxy session instance. In other words, once per server request. ## Additional context https://github.com/supabase/auth-js/issues/888 --- src/GoTrueClient.ts | 6 ++++-- test/GoTrueClient.test.ts | 16 ++++++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/GoTrueClient.ts b/src/GoTrueClient.ts index 88bedadcc..94247672c 100644 --- a/src/GoTrueClient.ts +++ b/src/GoTrueClient.ts @@ -1113,14 +1113,16 @@ export default class GoTrueClient { if (!hasExpired) { if (this.storage.isServer) { - const suppressWarning = this.suppressGetSessionWarning + let suppressWarning = this.suppressGetSessionWarning const proxySession: Session = new Proxy(currentSession, { - get(target: any, prop: string, receiver: any) { + 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 many 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) }, diff --git a/test/GoTrueClient.test.ts b/test/GoTrueClient.test.ts index ec5ac3d1a..9f8127639 100644 --- a/test/GoTrueClient.test.ts +++ b/test/GoTrueClient.test.ts @@ -943,7 +943,7 @@ describe('GoTrueClient with storageisServer = true', () => { expect(warnings.length).toEqual(0) }) - test('getSession() emits insecure warning when user object is accessed', async () => { + test('getSession() emits insecure warning, once per server client, when user object is accessed', async () => { const storage = memoryLocalStorageAdapter({ [STORAGE_KEY]: JSON.stringify({ access_token: 'jwt.accesstoken.signature', @@ -966,7 +966,7 @@ describe('GoTrueClient with storageisServer = true', () => { data: { session }, } = await client.getSession() - const user = session?.user // accessing the user object from getSession should emit a warning + 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( @@ -974,6 +974,18 @@ describe('GoTrueClient with storageisServer = true', () => { '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) }) test('getSession emits no warnings if getUser is called prior', async () => {