Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 15 additions & 16 deletions packages/core/auth-js/src/GoTrueClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
getAlgorithm,
getCodeChallengeAndMethod,
getItemAsync,
insecureUserWarningProxy,
isBrowser,
parseParametersFromURL,
removeItemAsync,
Expand Down Expand Up @@ -1600,22 +1601,20 @@ export default class GoTrueClient {
}
}

if (this.storage.isServer && currentSession.user) {
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
// Wrap the user object with a warning proxy on the server
// This warns when properties of the user are accessed, not when session.user itself is accessed
if (
this.storage.isServer &&
currentSession.user &&
!(currentSession.user as any).__isUserNotAvailableProxy
) {
const suppressWarningRef = { value: this.suppressGetSessionWarning }
currentSession.user = insecureUserWarningProxy(currentSession.user, suppressWarningRef)

// Update the client-level suppression flag when the proxy suppresses the warning
if (suppressWarningRef.value) {
this.suppressGetSessionWarning = true
}
}

return { data: { session: currentSession }, error: null }
Expand Down
44 changes: 44 additions & 0 deletions packages/core/auth-js/src/lib/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,50 @@ export function userNotAvailableProxy(): User {
})
}

/**
* Creates a proxy around a user object that warns when properties are accessed on the server.
* This is used to alert developers that using user data from getSession() on the server is insecure.
*
* @param user The actual user object to wrap
* @param suppressWarningRef An object with a 'value' property that controls warning suppression
* @returns A proxied user object that warns on property access
*/
export function insecureUserWarningProxy(user: User, suppressWarningRef: { value: boolean }): User {
return new Proxy(user, {
get: (target: any, prop: string | symbol, receiver: any) => {
// Allow internal checks without warning
if (prop === '__isInsecureUserWarningProxy') {
return true
}

// Preventative check for common problematic symbols during cloning/inspection
// These symbols might be accessed by structuredClone or other internal mechanisms
if (typeof prop === 'symbol') {
const sProp = prop.toString()
if (
sProp === 'Symbol(Symbol.toPrimitive)' ||
sProp === 'Symbol(Symbol.toStringTag)' ||
sProp === 'Symbol(util.inspect.custom)' ||
sProp === 'Symbol(nodejs.util.inspect.custom)'
) {
// Return the actual value for these symbols to allow proper inspection
return Reflect.get(target, prop, receiver)
}
}

// Emit warning on first property access
if (!suppressWarningRef.value && typeof prop === 'string') {
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.'
)
suppressWarningRef.value = true
}

return Reflect.get(target, prop, receiver)
},
})
}

/**
* Deep clones a JSON-serializable object using JSON.parse(JSON.stringify(obj)).
* Note: Only works for JSON-safe data.
Expand Down
155 changes: 146 additions & 9 deletions packages/core/auth-js/test/GoTrueClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1847,12 +1847,17 @@ describe('GoTrueClient with storageisServer = true', () => {
const client = new GoTrueClient({
storage,
})
await client.getSession()
const {
data: { session },
} = await client.getSession()

// Accessing session.user should not emit a warning
const user = session?.user
expect(user).not.toBeNull()
expect(warnings.length).toEqual(0)
})

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

const user = session?.user // accessing the user object from getSession should emit a warning the first time
// Accessing session.user itself should not emit a warning
const user = session?.user
expect(user).not.toBeNull()
expect(warnings.length).toEqual(0)

// Accessing a property of the user object should emit a warning the first time
const userId = user?.id
expect(userId).toEqual('random-user-id')
expect(warnings.length).toEqual(1)
expect(
warnings[0][0].startsWith(
'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()
// Accessing another property should not emit additional warnings
const userEmail = user?.email
expect(userEmail).toEqual('test@example.com')
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()
// Accessing properties in subsequent sessions should not emit warnings (suppression is client-wide)
const userId2 = session2?.user?.id
expect(userId2).toEqual('random-user-id')
expect(warnings.length).toEqual(1)
})

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

const sessionUser = session?.user // accessing the user object from getSession shouldn't emit a warning
expect(sessionUser).not.toBeNull()
// Accessing user properties from getSession shouldn't emit a warning after getUser() was called
const sessionUserId = session?.user?.id
expect(sessionUserId).not.toBeNull()
expect(warnings.length).toEqual(0)
})

test('getSession() with destructuring emits warning', async () => {
const storage = memoryLocalStorageAdapter({
[STORAGE_KEY]: JSON.stringify({
access_token: 'jwt.accesstoken.signature',
refresh_token: 'refresh-token',
token_type: 'bearer',
expires_in: 1000,
expires_at: Date.now() / 1000 + 1000,
user: {
id: 'random-user-id',
email: 'test@example.com',
role: 'user',
},
}),
})
storage.isServer = true

const client = new GoTrueClient({
storage,
})

const {
data: { session },
} = await client.getSession()

// Destructuring user properties should emit a warning
const { id, email } = session?.user || {}
expect(id).toEqual('random-user-id')
expect(email).toEqual('test@example.com')
expect(warnings.length).toEqual(1)
})

test('getSession() with spread operator emits warning', async () => {
const storage = memoryLocalStorageAdapter({
[STORAGE_KEY]: JSON.stringify({
access_token: 'jwt.accesstoken.signature',
refresh_token: 'refresh-token',
token_type: 'bearer',
expires_in: 1000,
expires_at: Date.now() / 1000 + 1000,
user: {
id: 'random-user-id',
email: 'test@example.com',
},
}),
})
storage.isServer = true

const client = new GoTrueClient({
storage,
})

const {
data: { session },
} = await client.getSession()

// Spread operator accesses properties, should emit a warning
const userData = { ...session?.user }
expect(userData.id).toEqual('random-user-id')
expect(warnings.length).toEqual(1)
})

test('getSession() with Object.keys() emits warning', async () => {
const storage = memoryLocalStorageAdapter({
[STORAGE_KEY]: JSON.stringify({
access_token: 'jwt.accesstoken.signature',
refresh_token: 'refresh-token',
token_type: 'bearer',
expires_in: 1000,
expires_at: Date.now() / 1000 + 1000,
user: {
id: 'random-user-id',
email: 'test@example.com',
},
}),
})
storage.isServer = true

const client = new GoTrueClient({
storage,
})

const {
data: { session },
} = await client.getSession()

// Object.keys() accesses properties, should emit a warning
const keys = Object.keys(session?.user || {})
expect(keys.length).toBeGreaterThan(0)
expect(warnings.length).toEqual(1)
})

test('getSession() with JSON.stringify() emits warning', async () => {
const storage = memoryLocalStorageAdapter({
[STORAGE_KEY]: JSON.stringify({
access_token: 'jwt.accesstoken.signature',
refresh_token: 'refresh-token',
token_type: 'bearer',
expires_in: 1000,
expires_at: Date.now() / 1000 + 1000,
user: {
id: 'random-user-id',
email: 'test@example.com',
},
}),
})
storage.isServer = true

const client = new GoTrueClient({
storage,
})

const {
data: { session },
} = await client.getSession()

// JSON.stringify() iterates over properties, should emit a warning
const serialized = JSON.stringify(session?.user)
expect(serialized).toContain('random-user-id')
expect(warnings.length).toEqual(1)
})

test('saveSession should overwrite the existing session', async () => {
const store = memoryLocalStorageAdapter()
const client = new GoTrueClient({
Expand Down
Loading