From 0d9d05f2c911427c2a5d86f9da0784be39da5a1b Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Thu, 18 Apr 2024 19:05:56 +0700 Subject: [PATCH 1/3] fix: show warning if user is accessed from session object on the server --- src/GoTrueClient.ts | 36 ++++++------------------------------ 1 file changed, 6 insertions(+), 30 deletions(-) diff --git a/src/GoTrueClient.ts b/src/GoTrueClient.ts index 888704219..c12f5d780 100644 --- a/src/GoTrueClient.ts +++ b/src/GoTrueClient.ts @@ -167,8 +167,6 @@ export default class GoTrueClient { protected logDebugMessages: boolean protected logger: (message: string, ...args: any[]) => void = console.log - protected insecureGetSessionWarningShown = false - /** * Create a new client for use in the browser. */ @@ -932,15 +930,6 @@ export default class GoTrueClient { }) }) - if (result.data && this.storage.isServer) { - if (!this.insecureGetSessionWarningShown) { - console.warn( - 'Using supabase.auth.getSession() is potentially insecure as it loads data directly from the storage medium (typically cookies) which may not be authentic. Prefer using supabase.auth.getUser() instead. To suppress this warning call supabase.auth.getUser() before you call supabase.auth.getSession().' - ) - this.insecureGetSessionWarningShown = true - } - } - return result } @@ -1120,26 +1109,18 @@ export default class GoTrueClient { if (!hasExpired) { if (this.storage.isServer) { - let user = currentSession.user - - delete (currentSession as any).user - - Object.defineProperty(currentSession, 'user', { - enumerable: true, - get: () => { - if (!(currentSession as any).__suppressUserWarning) { - // do not suppress this warning if insecureGetSessionWarningShown is true, as the data is still not authenticated + const proxySession: Session = new Proxy(currentSession, { + get(target: any, prop: string, receiver: any) { + if (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.' ) } - - return user - }, - set: (value) => { - user = value + return Reflect.get(target, prop, receiver) }, }) + currentSession = proxySession } return { data: { session: currentSession }, error: null } @@ -1174,11 +1155,6 @@ export default class GoTrueClient { return await this._getUser() }) - if (result.data && this.storage.isServer) { - // no longer emit the insecure warning for getSession() as the access_token is now authenticated - this.insecureGetSessionWarningShown = true - } - return result } From b7732fb1ff88837db538f2d2ea6f3e095e6abd4d Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Thu, 18 Apr 2024 19:06:09 +0700 Subject: [PATCH 2/3] chore: update eslint --- .eslintrc.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.eslintrc.json b/.eslintrc.json index 084528102..1b78689db 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -3,7 +3,7 @@ "browser": true, "es2021": true }, - "extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended"], + "extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended", "prettier"], "parser": "@typescript-eslint/parser", "parserOptions": { "ecmaVersion": 12, From 1643381fc342809f0e62ae7b5b2bea38599d5b4c Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Thu, 18 Apr 2024 23:37:36 +0700 Subject: [PATCH 3/3] chore: fix test --- test/GoTrueClient.test.ts | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/test/GoTrueClient.test.ts b/test/GoTrueClient.test.ts index 140107b4e..e86911a4e 100644 --- a/test/GoTrueClient.test.ts +++ b/test/GoTrueClient.test.ts @@ -927,7 +927,7 @@ describe('GoTrueClient with storageisServer = true', () => { warnings = [] }) - test('getSession() emits two insecure warnings', async () => { + test('getSession() emits no warnings', async () => { const storage = memoryLocalStorageAdapter({ [STORAGE_KEY]: JSON.stringify({ access_token: 'jwt.accesstoken.signature', @@ -945,26 +945,9 @@ describe('GoTrueClient with storageisServer = true', () => { const client = new GoTrueClient({ storage, }) + await client.getSession() - const { - data: { session }, - } = await client.getSession() - - console.log('User is ', session!.user!.id) - - const firstWarning = warnings[0] - const lastWarning = warnings[warnings.length - 1] - - expect( - firstWarning[0].startsWith( - 'Using supabase.auth.getSession() is potentially insecure as it loads data directly from the storage medium (typically cookies) which may not be authentic' - ) - ).toEqual(true) - expect( - lastWarning[0].startsWith( - 'Using the user object as returned from supabase.auth.getSession() ' - ) - ).toEqual(true) + expect(warnings.length).toEqual(0) }) test('getSession() emits one insecure warning', async () => {