Skip to content

Commit 0a76a61

Browse files
authored
fix(auth): move session warning proxy from session to user object (#1817)
1 parent 4c76af8 commit 0a76a61

File tree

3 files changed

+205
-25
lines changed

3 files changed

+205
-25
lines changed

packages/core/auth-js/src/GoTrueClient.ts

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
getAlgorithm,
3939
getCodeChallengeAndMethod,
4040
getItemAsync,
41+
insecureUserWarningProxy,
4142
isBrowser,
4243
parseParametersFromURL,
4344
removeItemAsync,
@@ -1600,22 +1601,20 @@ export default class GoTrueClient {
16001601
}
16011602
}
16021603

1603-
if (this.storage.isServer && currentSession.user) {
1604-
let suppressWarning = this.suppressGetSessionWarning
1605-
const proxySession: Session = new Proxy(currentSession, {
1606-
get: (target: any, prop: string, receiver: any) => {
1607-
if (!suppressWarning && prop === 'user') {
1608-
// only show warning when the user object is being accessed from the server
1609-
console.warn(
1610-
'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.'
1611-
)
1612-
suppressWarning = true // keeps this proxy instance from logging additional warnings
1613-
this.suppressGetSessionWarning = true // keeps this client's future proxy instances from warning
1614-
}
1615-
return Reflect.get(target, prop, receiver)
1616-
},
1617-
})
1618-
currentSession = proxySession
1604+
// Wrap the user object with a warning proxy on the server
1605+
// This warns when properties of the user are accessed, not when session.user itself is accessed
1606+
if (
1607+
this.storage.isServer &&
1608+
currentSession.user &&
1609+
!(currentSession.user as any).__isUserNotAvailableProxy
1610+
) {
1611+
const suppressWarningRef = { value: this.suppressGetSessionWarning }
1612+
currentSession.user = insecureUserWarningProxy(currentSession.user, suppressWarningRef)
1613+
1614+
// Update the client-level suppression flag when the proxy suppresses the warning
1615+
if (suppressWarningRef.value) {
1616+
this.suppressGetSessionWarning = true
1617+
}
16191618
}
16201619

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

packages/core/auth-js/src/lib/helpers.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,50 @@ export function userNotAvailableProxy(): User {
407407
})
408408
}
409409

410+
/**
411+
* Creates a proxy around a user object that warns when properties are accessed on the server.
412+
* This is used to alert developers that using user data from getSession() on the server is insecure.
413+
*
414+
* @param user The actual user object to wrap
415+
* @param suppressWarningRef An object with a 'value' property that controls warning suppression
416+
* @returns A proxied user object that warns on property access
417+
*/
418+
export function insecureUserWarningProxy(user: User, suppressWarningRef: { value: boolean }): User {
419+
return new Proxy(user, {
420+
get: (target: any, prop: string | symbol, receiver: any) => {
421+
// Allow internal checks without warning
422+
if (prop === '__isInsecureUserWarningProxy') {
423+
return true
424+
}
425+
426+
// Preventative check for common problematic symbols during cloning/inspection
427+
// These symbols might be accessed by structuredClone or other internal mechanisms
428+
if (typeof prop === 'symbol') {
429+
const sProp = prop.toString()
430+
if (
431+
sProp === 'Symbol(Symbol.toPrimitive)' ||
432+
sProp === 'Symbol(Symbol.toStringTag)' ||
433+
sProp === 'Symbol(util.inspect.custom)' ||
434+
sProp === 'Symbol(nodejs.util.inspect.custom)'
435+
) {
436+
// Return the actual value for these symbols to allow proper inspection
437+
return Reflect.get(target, prop, receiver)
438+
}
439+
}
440+
441+
// Emit warning on first property access
442+
if (!suppressWarningRef.value && typeof prop === 'string') {
443+
console.warn(
444+
'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.'
445+
)
446+
suppressWarningRef.value = true
447+
}
448+
449+
return Reflect.get(target, prop, receiver)
450+
},
451+
})
452+
}
453+
410454
/**
411455
* Deep clones a JSON-serializable object using JSON.parse(JSON.stringify(obj)).
412456
* Note: Only works for JSON-safe data.

packages/core/auth-js/test/GoTrueClient.test.ts

Lines changed: 146 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1847,12 +1847,17 @@ describe('GoTrueClient with storageisServer = true', () => {
18471847
const client = new GoTrueClient({
18481848
storage,
18491849
})
1850-
await client.getSession()
1850+
const {
1851+
data: { session },
1852+
} = await client.getSession()
18511853

1854+
// Accessing session.user should not emit a warning
1855+
const user = session?.user
1856+
expect(user).not.toBeNull()
18521857
expect(warnings.length).toEqual(0)
18531858
})
18541859

1855-
test('getSession() emits insecure warning, once per server client, when user object is accessed', async () => {
1860+
test('getSession() emits insecure warning, once per server client, when user properties are accessed', async () => {
18561861
const storage = memoryLocalStorageAdapter({
18571862
[STORAGE_KEY]: JSON.stringify({
18581863
access_token: 'jwt.accesstoken.signature',
@@ -1862,6 +1867,7 @@ describe('GoTrueClient with storageisServer = true', () => {
18621867
expires_at: Date.now() / 1000 + 1000,
18631868
user: {
18641869
id: 'random-user-id',
1870+
email: 'test@example.com',
18651871
},
18661872
}),
18671873
})
@@ -1875,25 +1881,33 @@ describe('GoTrueClient with storageisServer = true', () => {
18751881
data: { session },
18761882
} = await client.getSession()
18771883

1878-
const user = session?.user // accessing the user object from getSession should emit a warning the first time
1884+
// Accessing session.user itself should not emit a warning
1885+
const user = session?.user
18791886
expect(user).not.toBeNull()
1887+
expect(warnings.length).toEqual(0)
1888+
1889+
// Accessing a property of the user object should emit a warning the first time
1890+
const userId = user?.id
1891+
expect(userId).toEqual('random-user-id')
18801892
expect(warnings.length).toEqual(1)
18811893
expect(
18821894
warnings[0][0].startsWith(
18831895
'Using the user object as returned from supabase.auth.getSession() '
18841896
)
18851897
).toEqual(true)
18861898

1887-
const user2 = session?.user // accessing the user object further should not emit a warning
1888-
expect(user2).not.toBeNull()
1899+
// Accessing another property should not emit additional warnings
1900+
const userEmail = user?.email
1901+
expect(userEmail).toEqual('test@example.com')
18891902
expect(warnings.length).toEqual(1)
18901903

18911904
const {
18921905
data: { session: session2 },
18931906
} = await client.getSession() // create new proxy instance
18941907

1895-
const user3 = session2?.user // accessing the user object in subsequent proxy instances, for this client, should not emit a warning
1896-
expect(user3).not.toBeNull()
1908+
// Accessing properties in subsequent sessions should not emit warnings (suppression is client-wide)
1909+
const userId2 = session2?.user?.id
1910+
expect(userId2).toEqual('random-user-id')
18971911
expect(warnings.length).toEqual(1)
18981912
})
18991913

@@ -1921,11 +1935,134 @@ describe('GoTrueClient with storageisServer = true', () => {
19211935
data: { session },
19221936
} = await client.getSession()
19231937

1924-
const sessionUser = session?.user // accessing the user object from getSession shouldn't emit a warning
1925-
expect(sessionUser).not.toBeNull()
1938+
// Accessing user properties from getSession shouldn't emit a warning after getUser() was called
1939+
const sessionUserId = session?.user?.id
1940+
expect(sessionUserId).not.toBeNull()
19261941
expect(warnings.length).toEqual(0)
19271942
})
19281943

1944+
test('getSession() with destructuring emits warning', async () => {
1945+
const storage = memoryLocalStorageAdapter({
1946+
[STORAGE_KEY]: JSON.stringify({
1947+
access_token: 'jwt.accesstoken.signature',
1948+
refresh_token: 'refresh-token',
1949+
token_type: 'bearer',
1950+
expires_in: 1000,
1951+
expires_at: Date.now() / 1000 + 1000,
1952+
user: {
1953+
id: 'random-user-id',
1954+
email: 'test@example.com',
1955+
role: 'user',
1956+
},
1957+
}),
1958+
})
1959+
storage.isServer = true
1960+
1961+
const client = new GoTrueClient({
1962+
storage,
1963+
})
1964+
1965+
const {
1966+
data: { session },
1967+
} = await client.getSession()
1968+
1969+
// Destructuring user properties should emit a warning
1970+
const { id, email } = session?.user || {}
1971+
expect(id).toEqual('random-user-id')
1972+
expect(email).toEqual('test@example.com')
1973+
expect(warnings.length).toEqual(1)
1974+
})
1975+
1976+
test('getSession() with spread operator emits warning', async () => {
1977+
const storage = memoryLocalStorageAdapter({
1978+
[STORAGE_KEY]: JSON.stringify({
1979+
access_token: 'jwt.accesstoken.signature',
1980+
refresh_token: 'refresh-token',
1981+
token_type: 'bearer',
1982+
expires_in: 1000,
1983+
expires_at: Date.now() / 1000 + 1000,
1984+
user: {
1985+
id: 'random-user-id',
1986+
email: 'test@example.com',
1987+
},
1988+
}),
1989+
})
1990+
storage.isServer = true
1991+
1992+
const client = new GoTrueClient({
1993+
storage,
1994+
})
1995+
1996+
const {
1997+
data: { session },
1998+
} = await client.getSession()
1999+
2000+
// Spread operator accesses properties, should emit a warning
2001+
const userData = { ...session?.user }
2002+
expect(userData.id).toEqual('random-user-id')
2003+
expect(warnings.length).toEqual(1)
2004+
})
2005+
2006+
test('getSession() with Object.keys() emits warning', async () => {
2007+
const storage = memoryLocalStorageAdapter({
2008+
[STORAGE_KEY]: JSON.stringify({
2009+
access_token: 'jwt.accesstoken.signature',
2010+
refresh_token: 'refresh-token',
2011+
token_type: 'bearer',
2012+
expires_in: 1000,
2013+
expires_at: Date.now() / 1000 + 1000,
2014+
user: {
2015+
id: 'random-user-id',
2016+
email: 'test@example.com',
2017+
},
2018+
}),
2019+
})
2020+
storage.isServer = true
2021+
2022+
const client = new GoTrueClient({
2023+
storage,
2024+
})
2025+
2026+
const {
2027+
data: { session },
2028+
} = await client.getSession()
2029+
2030+
// Object.keys() accesses properties, should emit a warning
2031+
const keys = Object.keys(session?.user || {})
2032+
expect(keys.length).toBeGreaterThan(0)
2033+
expect(warnings.length).toEqual(1)
2034+
})
2035+
2036+
test('getSession() with JSON.stringify() emits warning', async () => {
2037+
const storage = memoryLocalStorageAdapter({
2038+
[STORAGE_KEY]: JSON.stringify({
2039+
access_token: 'jwt.accesstoken.signature',
2040+
refresh_token: 'refresh-token',
2041+
token_type: 'bearer',
2042+
expires_in: 1000,
2043+
expires_at: Date.now() / 1000 + 1000,
2044+
user: {
2045+
id: 'random-user-id',
2046+
email: 'test@example.com',
2047+
},
2048+
}),
2049+
})
2050+
storage.isServer = true
2051+
2052+
const client = new GoTrueClient({
2053+
storage,
2054+
})
2055+
2056+
const {
2057+
data: { session },
2058+
} = await client.getSession()
2059+
2060+
// JSON.stringify() iterates over properties, should emit a warning
2061+
const serialized = JSON.stringify(session?.user)
2062+
expect(serialized).toContain('random-user-id')
2063+
expect(warnings.length).toEqual(1)
2064+
})
2065+
19292066
test('saveSession should overwrite the existing session', async () => {
19302067
const store = memoryLocalStorageAdapter()
19312068
const client = new GoTrueClient({

0 commit comments

Comments
 (0)