Skip to content

Commit f7fa563

Browse files
committed
feat: improved insecure getSession() warning
1 parent 9748dd9 commit f7fa563

File tree

3 files changed

+167
-17
lines changed

3 files changed

+167
-17
lines changed

src/GoTrueClient.ts

+4-15
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
supportsLocalStorage,
3737
parseParametersFromURL,
3838
getCodeChallengeAndMethod,
39+
userNotAvailableProxy,
3940
} from './lib/helpers'
4041
import { localStorageAdapter, memoryLocalStorageAdapter } from './lib/local-storage'
4142
import { polyfillGlobalThis } from './lib/polyfills'
@@ -1120,21 +1121,9 @@ export default class GoTrueClient {
11201121

11211122
if (!hasExpired) {
11221123
if (this.storage.isServer) {
1123-
let suppressWarning = this.suppressGetSessionWarning
1124-
const proxySession: Session = new Proxy(currentSession, {
1125-
get: (target: any, prop: string, receiver: any) => {
1126-
if (!suppressWarning && prop === 'user') {
1127-
// only show warning when the user object is being accessed from the server
1128-
console.warn(
1129-
'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.'
1130-
)
1131-
suppressWarning = true // keeps this proxy instance from logging additional warnings
1132-
this.suppressGetSessionWarning = true // keeps this client's future proxy instances from warning
1133-
}
1134-
return Reflect.get(target, prop, receiver)
1135-
},
1136-
})
1137-
currentSession = proxySession
1124+
currentSession.user = userNotAvailableProxy(
1125+
'User object comes from insecure storage and may not be authentic. Call getUser() instead to prevent security issues.'
1126+
)
11381127
}
11391128

11401129
return { data: { session: currentSession }, error: null }

src/lib/helpers.ts

+75-1
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,26 @@
11
import { API_VERSION_HEADER_NAME } from './constants'
2-
import { SupportedStorage } from './types'
2+
import { SupportedStorage, User } from './types'
33

44
export function expiresAt(expiresIn: number) {
55
const timeNow = Math.round(Date.now() / 1000)
66
return timeNow + expiresIn
77
}
88

99
export function uuid() {
10+
if (
11+
'crypto' in globalThis &&
12+
typeof globalThis.crypto === 'object' &&
13+
'randomUUID' in globalThis.crypto &&
14+
typeof globalThis.crypto.randomUUID === 'function'
15+
) {
16+
try {
17+
return globalThis.crypto.randomUUID()
18+
} catch (e: any) {
19+
// ignore and fall back to below implementation, as crypto.randomUUID()
20+
// only works in secure contexts
21+
}
22+
}
23+
1024
return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function (c) {
1125
const r = (Math.random() * 16) | 0,
1226
v = c == 'x' ? r : (r & 0x3) | 0x8
@@ -344,3 +358,63 @@ export function parseResponseAPIVersion(response: Response) {
344358
return null
345359
}
346360
}
361+
362+
const warningSymbol = Symbol('WARNING')
363+
364+
class UnavailableObject {
365+
constructor(reason: string) {
366+
;(this as any)[warningSymbol] = reason
367+
}
368+
369+
toString() {
370+
return JSON.stringify({ WARNING: (this as any)[warningSymbol] })
371+
}
372+
}
373+
374+
const PROPERTY_WARNINGS_SHOWN: { [reason: string]: boolean } = {}
375+
376+
export function userNotAvailableProxy(reason: string): User {
377+
const target = new UnavailableObject(
378+
`@supabase/auth-js: Attempting to access a user object which is not available. Reason: ${reason}`
379+
)
380+
381+
return new Proxy(target as User, {
382+
get: (target: any, prop: PropertyKey, receiver: any) => {
383+
if (typeof prop !== 'string' || prop in target) {
384+
return Reflect.get(target, prop, receiver)
385+
}
386+
387+
let returnValue: any = undefined
388+
switch (prop) {
389+
case 'id':
390+
case 'created_at':
391+
case 'aud':
392+
returnValue = '@@@ not-available @@@'
393+
break
394+
395+
case 'app_metadata':
396+
case 'user_metadata':
397+
returnValue = new UnavailableObject(
398+
`@supabase/auth-js: User property object "${prop}" is not available. Reason: ${reason}`
399+
)
400+
break
401+
}
402+
403+
if (!PROPERTY_WARNINGS_SHOWN[reason]) {
404+
PROPERTY_WARNINGS_SHOWN[reason] = true
405+
406+
console.warn(
407+
`@supabase/auth-js: Accessing the "${prop}" (or any other) property of the user object under session is not supported, returned value will be ${
408+
returnValue === undefined
409+
? 'undefined'
410+
: returnValue instanceof UnavailableObject
411+
? 'empty object'
412+
: JSON.stringify(returnValue)
413+
}. Reason: ${reason}`
414+
)
415+
}
416+
417+
return returnValue
418+
},
419+
})
420+
}

test/helpers.test.ts

+88-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
import { parseParametersFromURL, parseResponseAPIVersion } from '../src/lib/helpers'
1+
import {
2+
parseParametersFromURL,
3+
parseResponseAPIVersion,
4+
userNotAvailableProxy,
5+
uuid,
6+
} from '../src/lib/helpers'
27

38
describe('parseParametersFromURL', () => {
49
it('should parse parameters from a URL with query params only', () => {
@@ -71,3 +76,85 @@ describe('parseResponseAPIVersion', () => {
7176
})
7277
})
7378
})
79+
80+
describe('uuid', () => {
81+
if ('crypto' in globalThis) {
82+
// nodejs 18, 20 don't have crypto
83+
84+
it('should generate a uuid when crypto.randomUUID() throws an error', () => {
85+
const originalRandomUUID = crypto.randomUUID
86+
87+
try {
88+
crypto.randomUUID = () => {
89+
throw new Error('Fail for test')
90+
}
91+
92+
expect(typeof uuid()).toEqual('string')
93+
} finally {
94+
crypto.randomUUID = originalRandomUUID
95+
}
96+
})
97+
98+
it('should generate a uuid with crypto.randomUUID()', () => {
99+
const originalRandomUUID = crypto.randomUUID
100+
101+
try {
102+
crypto.randomUUID = () => {
103+
return 'random-uuid'
104+
}
105+
106+
expect(uuid()).toEqual('random-uuid')
107+
} finally {
108+
crypto.randomUUID = originalRandomUUID
109+
}
110+
})
111+
}
112+
113+
it('should generate a uuid', () => {
114+
expect(typeof uuid()).toEqual('string')
115+
})
116+
})
117+
118+
describe('userNotAvailableProxy', () => {
119+
it('should show a warning when calling toString()', () => {
120+
expect(`${userNotAvailableProxy('REASON-0')}`).toMatchInlineSnapshot(
121+
`"{\\"WARNING\\":\\"@supabase/auth-js: Attempting to access a user object which is not available. Reason: REASON-0\\"}"`
122+
)
123+
})
124+
125+
it('should show a warning when calling toString()', () => {
126+
const originalWarn = console.warn
127+
128+
try {
129+
let numberOfWarnings = 0
130+
console.warn = (...args: any[]) => {
131+
expect(args).toMatchInlineSnapshot(`
132+
Array [
133+
"@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",
134+
]
135+
`)
136+
numberOfWarnings += 1
137+
}
138+
139+
const object = userNotAvailableProxy('REASON-1')
140+
expect(object.id).toMatchInlineSnapshot(`"@@@ not-available @@@"`)
141+
expect(object.created_at).toMatchInlineSnapshot(`"@@@ not-available @@@"`)
142+
expect(object.aud).toMatchInlineSnapshot(`"@@@ not-available @@@"`)
143+
expect(object.app_metadata).toMatchInlineSnapshot(`
144+
UnavailableObject {
145+
Symbol(WARNING): "@supabase/auth-js: User property object \\"app_metadata\\" is not available. Reason: REASON-1",
146+
}
147+
`)
148+
expect(object.user_metadata).toMatchInlineSnapshot(`
149+
UnavailableObject {
150+
Symbol(WARNING): "@supabase/auth-js: User property object \\"user_metadata\\" is not available. Reason: REASON-1",
151+
}
152+
`)
153+
expect(object.email).toMatchInlineSnapshot(`undefined`)
154+
155+
expect(numberOfWarnings).toEqual(1)
156+
} finally {
157+
console.warn = originalWarn
158+
}
159+
})
160+
})

0 commit comments

Comments
 (0)