Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update session warning #879

Merged
merged 3 commits into from
Apr 18, 2024
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
2 changes: 1 addition & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
36 changes: 6 additions & 30 deletions src/GoTrueClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -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
}

Expand Down
23 changes: 3 additions & 20 deletions test/GoTrueClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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 () => {
Expand Down
Loading