user object warning logged, even when not touching session.user
#888
Comments
|
Someone having this issue when:
|
|
It's likely the other two warnings, that I couldn't figure out how they are triggering, are triggered when SvelteKit checks if passed values are POJOs, in certain situations. See #873 (comment) |
|
Thanks @kangmingtay. I suspect that PR will resolve issues for some people - specifically the ones exclusively experiencing this when calling things like As an aside, how do I test an RC? I added an override in my package.json, and after doing |
|
@kangmingtay looks like there is something wrong with the build process for RCs. If you checkout the code on npm, you'll see the PR code is not there. |
|
@j4w8n yeah your analysis is right, any method that implicitly accesses the user property in the session object will trigger the warning log. i think we can implement your suggestion here so the warning is only logged once per proxy session, but will need some time to test this out since we're currently quite tight on bandwidth |
|
Until this issue is resolved, and perhaps even after that, I've tweaked my demo app - v0.3.0 - to where I no longer receive any warnings. My solution is done in a legitimate way, which checks all of the security boxes. The code in hooks.server.ts verifies the JWT and uses it's decoded data to craft a validated session. This session will pass any internal auth-js checks, as well as type checks. |
|
I think the suggestion here from @j4w8n is great but I'd also like a way to suppress this warning completely in production build. I don't want this to be logging inside of my application terminal when its being run in production mode. |
## What kind of change does this PR introduce? Bug fix. ## What is the current behavior? A call to `getSession()`, when using server storage, logs a warning every time `session.user` is accessed. This is causing a lot of people to see multiple consecutive logs to the console - especially for SvelteKit users. ## What is the new behavior? Ensures that accessing `session.user`, from a `getSession()` call, only logs the warning once per Proxy session instance. In other words, once per server request. ## Additional context #888
|
any update? |
* Sveltekit 2 migration! Mostly just running `npx svelte-migrate@latest sveltekit-2` and fixing package conflicts. Use @supabase/auth-helpers-sveltekit to ^0.11 to avoid supabase/auth-js#888
|
Still logging, try different way to suppress it too, seems nothing works vite, sveltekit |
|
any update?? |
|
Any Updates? |
|
For all folks using SvelteKit, here's a robust solution which both removes the annoying logs and avoid a network call to get user's data. |
|
I just followed the official SvelteKit example, but without the session object. This gets rid of the log. 99% of the time, you just need the user object if there is one. J |
I think part of the issue is that the docs have multiple examples with SvelteKit and they are not all updated to reflect the current "correct" methods - and, meaning no offense, comments like this just add to the confusion because it all depends on which example you followed.
Can you elaborate on this? |
|
@vehm - I'm not sure what you mean. There is only one example page that I'm aware of: https://supabase.com/docs/guides/getting-started/tutorials/with-sveltekit But to be clear, here is my hooks code. You usually only need event.locals.safeGetSession = async () => {
const {
data: { session },
} = await event.locals.supabase.auth.getSession();
if (!session) {
return { user: null };
}
const {
data: { user },
error,
} = await event.locals.supabase.auth.getUser();
if (error) {
// JWT validation has failed
return { user: null };
}
return { user };
};That being said there is still the stupid log even when using J |
Here are the three I was referring to (including the one you linked to):
Essentially, there are a few guides that all approach things a little differently and it's difficult to know which of these is the "correct" or "up-to-date" method. |
|
They all have the same hooks file unless I'm missing something? J |
The first linked guide utilizes an authGuard + They were most inconsistent when they made the change from |
|
Which one should I use so that I don't see the error? I'm also using #888 (comment) approach and thought this is the way (checking the user, then the session) After seeing https://supabase.com/docs/guides/getting-started/tutorials/with-sveltekit example, session and user is used reversed (calling session first, then obtain the user) The error is so annoying 🫣 |
|
Could we just get a "markUserAsVerifed" method as a interm solution if this fix is going to take much longer? To current state of svelte+supabase is pretty frustrating. Non SSR auth has been deprecated, and SSR auth hits constant security warnings (even when doing things correctly). |
|
@wiesson, there are no official examples which don't log the warning when using SvelteKit. @scosman, yeah, bit of a mess right now, but I doubt they're going to introduce a new method before a fix happens. Maybe there will be one that comes along with the fixes? I don't know the full roadmap of how they're addressing this, but the new ssr update is in RC right now; and asymmetric JWTs are coming, plus an augmented However, in my opinion, more changes are needed to address the warning being logged when using things like |
|
Is it okay to stay on |
Since it's more of a logging nuisance than an actual issue, I'd consider staying on 0.3.0. But stick with whatever is working for you. |
I'm in the same boat. I implemented safeGetSession() as per the documentation but I'm getting overrun with these logs. I hope the RC people are talking about will resolve that? |
This is really a nightmare. I cannot even console.log on server to quickly look up a behavior. |
The warning is unnecessary imho, could be a warning in the docs, alongside the guides or a comment alongside the code. The warning makes server log debugging nightmare. I can't figure out what to read anymore. I presume it even makes the server slower over time as well. |
|
is supabase dont give a sht with these? |
|
this spam msg hit my productivity already |
|
For whomever that can help - while they fix this I setup a vite vite plugin that just suppresses this message. In vite.config.ts import { sveltekit } from '@sveltejs/kit/vite';
import { defineConfig } from 'vitest/config';
// myErrorFilterPlugin.ts
import type { Plugin } from 'vite';
export function myErrorFilterPlugin(): Plugin {
return {
name: 'error-filter-plugin',
configureServer(server) {
const filterMessage =
'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.';
// Intercept console.warn
const originalWarn = console.warn;
console.warn = (...args: any[]) => {
if (args[0] && typeof args[0] === 'string' && args[0].includes(filterMessage)) {
return;
}
originalWarn(...args);
};
// Intercept console.log
const originalLog = console.log;
console.log = (...args: any[]) => {
if (args[0] && typeof args[0] === 'string' && args[0].includes(filterMessage)) {
return;
}
originalLog(...args);
};
}
};
}
export default defineConfig({
plugins: [sveltekit(), myErrorFilterPlugin()],
test: {
include: ['src/**/*.{test,spec}.{js,ts}']
}
}); |
|
We can use #895 to suppress the warning by setting the protected property // @ts-expect-error - suppressGetSessionWarning is not part of the official API
event.locals.supabase.auth.suppressGetSessionWarning = true;Here's my hooks.server.ts: import { PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY } from '$env/static/public';
import { createServerClient } from '@supabase/ssr';
import type { Handle } from '@sveltejs/kit';
export const handle: Handle = async ({ event, resolve }) => {
event.locals.supabase = createServerClient(PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY, {
cookies: {
get: (key) => event.cookies.get(key),
/**
* Note: You have to add the `path` variable to the
* set and remove method due to sveltekit's cookie API
* requiring this to be set, setting the path to `/`
* will replicate previous/standard behaviour (https://kit.svelte.dev/docs/types#public-types-cookies)
*/
set: (key, value, options) => {
event.cookies.set(key, value, { ...options, path: '/' });
},
remove: (key, options) => {
event.cookies.delete(key, { ...options, path: '/' });
},
},
});
if ('suppressGetSessionWarning' in event.locals.supabase.auth) {
// @ts-expect-error - suppressGetSessionWarning is not part of the official API
event.locals.supabase.auth.suppressGetSessionWarning = true;
} else {
console.warn(
'SupabaseAuthClient#suppressGetSessionWarning was removed. See https://github.com/supabase/auth-js/issues/888.',
);
}
/**
* Unlike `supabase.auth.getSession()`, which returns the session _without_
* validating the JWT, this function also calls `getUser()` to validate the
* JWT before returning the session.
*/
event.locals.safeGetSession = async () => {
const {
data: { session },
} = await event.locals.supabase.auth.getSession();
if (!session) {
return { session: null, user: null };
}
const {
data: { user },
error,
} = await event.locals.supabase.auth.getUser();
if (error) {
// JWT validation has failed
return { session: null, user: null };
}
return { session, user };
};
return resolve(event, {
filterSerializedResponseHeaders(name) {
return name === 'content-range' || name === 'x-supabase-api-version';
},
});
}; |
|
In SvelteKit I am using this setup and it seems to work ok without any warning logs: Edit: Unfortunatelly there are still some cases when the warning log is fired (e.g. internal Supabase methods such as // hooks.server.js
import { PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY } from '$env/static/public'
import { createServerClient } from '@supabase/ssr'
export const handle = async ({ event, resolve }) => {
event.locals.supabaseServerClient = createServerClient(PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY, {
cookies: {
getAll() {
return event.cookies.getAll()
},
setAll(cookiesToSet) {
cookiesToSet.forEach(({ name, value, options }) =>
event.cookies.set(name, value, { ...options,path: '/' })
)
},
},
})
const getSessionAndUser = async () => {
const { data: { session } } = await event.locals.supabaseServerClient.auth.getSession()
if (!session) {
return {
session: null,
user: null
}
}
const { data: { user }, error } = await event.locals.supabaseServerClient.auth.getUser()
if (error) {
// JWT validation has failed
return {
session: null,
user: null
}
}
delete session.user
const sessionWithUserFromUser = { ...session, user: {...user} }
return { session: sessionWithUserFromUser, user }
}
const { session, user } = await getSessionAndUser()
event.locals.session = session
event.locals.user = user
return resolve(event, {
filterSerializedResponseHeaders(name) {
return name === 'content-range' || name === 'x-supabase-api-version'
},
})
}// +layout.server.js
export const load = async (event) => {
// covering the case when the user was deleted from the database, the cookie needs to be deleted in the browser
if (event.locals.session == null) {
event.cookies.delete(event.locals.supabaseServerClient.storageKey, { path: '/' });
}
return {
session: event.locals.session,
user: event.locals.user
};
};// +layout.js
import { PUBLIC_SUPABASE_ANON_KEY, PUBLIC_SUPABASE_URL } from '$env/static/public'
import { createBrowserClient, createServerClient, isBrowser } from '@supabase/ssr'
export const load = async ({ fetch, data, depends }) => {
depends('supabase:auth')
const supabase = isBrowser()
? createBrowserClient(PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY, {
global: {
fetch,
},
})
: createServerClient(PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY, {
global: {
fetch,
},
cookies: {
getAll() {
return data.cookies
},
},
})
const session = isBrowser()
? (await supabase.auth.getSession()).data.session
: data.session
return {
supabase,
session,
user: data.user
}
} |
…/auth/server-side/sveltekit Supress warnings that are not needed: supabase/auth-js#888 (comment)
|
So it turns out you do need the So, the error persists. It's been 6 months, other issues were pre-maturely closed, and this issue is not fixed! J |
|
@Hugo-Dz @j4w8n if |
|
@juniorforlife, in my opinion, for two reasons.
|
|
@j4w8n Thanks! From what I see in the example code, the only reason session is needed is it's used in onAuthStateChange to check expiration date. I think it's not necessary if we already check getUser in hooks.server.ts right? |
Depends on the needs of your app. |
|
If you need to get custom claims, you will also need the session. Custom Claims are good to get user information you don't want to continuously refetch on every request (username, display name, role, etc). Ironically, if you have to verify the session on every request via Also, there are still cases where you get the "getSession warning" even when you don't return a session at all like The whole process is flawed and should just be handled automatically with one function. Sigh. J |
|
I just blindly verify the session locally using jwt.verify just to get rid of this insane spamming satanic message. well it turns out this way much faster since supabase giving you jwt secret key. |
|
Are there any in-progress fixes for this? I'd like to decide whether to use SB for upcoming projects, and I see no movement here. |
Not that I'm aware of. You'll be fine though, since it's just a console warning; it doesn't inhibit any functionality. Mainly just annoying, especially when you're doing everything correctly. |
|
@StevenStavrakis - I wouldn't let this deter you from using Supabase. Every software you use has tradeoffs and temporary small bugs. It does look like the whole process is getting replaced eventually with "asymmetric keys." although there doesn't seem to be a timeline: J |
|
Asymmetric JWTs are supposed to land by the end of the year. Announcement: Some implementation details are in here: |
I came here because this warning made it seem like I must be using Supabase incorrectly or doing something insecure with it. |
You'll be good by following that SvelteKit guide. There will still be some warnings because of how SvelteKit checks some things, but no security issues there. |
Bug report
Describe the bug
With [email protected] and the ssr auth helper package, after a user logs in, the below warning is logged to the server console five times, with a fairly minimal SvelteKit app. This happens despite the fact that none of my code is calling
session.user, nor am I destructuring the user property fromsession, or destructuring with...rest-type syntax.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.
To Reproduce
Steps to reproduce the behavior, please provide code snippets or a repository:
https://github.com/j4w8n/getsession-warning
Expected behavior
The warning should not log when dev code doesn't explicitly call
session.user.Root Cause
I figured out why the first three logs happen: the Supabase client's server-side storage, from +layout.ts, is returning stringified JSON. You can see this in the docs.
JSON.stringify()causes each property ofsessionto be touched during the stringify process, which inevitably callssession.user. So, each time_useSession()is called, whether implicitly through dev code likegetSession()or explicitly with internal auth-js processes, the warning is being logged.I can't figure out where the final two warning logs are coming from. Could be from the same client initialization as above, but perhaps whatever triggers it is getting run asynchronously, so it's logging later.
Even if this warning wasn't an issue in the context of ssr, it could still be problematic, as the auth-js client itself calls
session.userwhen updating a user, setting a session, refreshing a session, and more - at least some of which could theoretically happen server-side.System information
Additional context
#873
#873 (comment)
https://tc39.es/ecma262/multipage/structured-data.html#sec-json.stringify
https://tc39.es/ecma262/multipage/structured-data.html#sec-serializejsonproperty
https://github.com/supabase/auth-js/blob/master/src/GoTrueClient.ts#L1111-L1124
The text was updated successfully, but these errors were encountered: