-
-
Notifications
You must be signed in to change notification settings - Fork 166
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
Still having getSession warning whenever _saveSession is called #912
Comments
Have you heard back about this? |
@larbish, you got any news about this issue? |
No news about it so far... @thorwebdev could someone have a check at this please? I'd love to merge my PR in the module and use |
We'll check this as soon as we can. Any external help in tracking down where the use comes from would help speed it up. |
Looks like the code calls |
I am seeing this constantly right now ever since updating to SSR 0.4.0 and following the documentation. Going back to SSR 0.3.0 this seems to git rid of the messages. |
Same here, running:
Getting:
Please let me know how I can support to resolve this! |
If you are calling |
I'm not calling or using A great example is to update a user or email: const { error } = await supabase
.auth
.updateUser({
email
}); Nothing I do will get rid of the message in this case. Any fixes? I can call J |
Only suppressing the warning will stop that right now. I know there are some issues where people have posted how to do that. I have not implemented any. |
I don't understand why this isn't fixed in the core code... this warning has created millions of wasted hours by supabase users. J |
I don't either. I created #888 for this, and they tried to cut down on the repetitiveness, but I'm not sure how much effort there has been to eliminate or deprecate the warning altogether. |
Until a fix is out, the only way to eliminate warnings is replace the // workaround for user object use message
const newSession: Omit<Session, "user"> & { user?: Session["user"] } = session;
delete newSession.user;
return { session: Object.assign({}, newSession, { user }), user }; |
@fnimick - I don't even use the import {
PUBLIC_SUPABASE_ANON_KEY,
PUBLIC_SUPABASE_URL,
} from "$env/static/public";
import type { Database } from "$lib/database.types";
import { createServerClient } from "@supabase/ssr";
import type { Handle } from "@sveltejs/kit";
export const handle: Handle = async ({ event, resolve }) => {
event.locals.supabase = createServerClient<Database>(
PUBLIC_SUPABASE_URL,
PUBLIC_SUPABASE_ANON_KEY,
{
cookies: {
getAll: () => event.cookies.getAll(),
setAll: (cookiesToSet) => {
cookiesToSet.forEach(({ name, value, options }) => {
event.cookies.set(name, value, {
...options,
path: "/",
});
});
},
},
},
);
event.locals.safeGetUser = 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) {
return { user: null };
}
return { user };
};
return resolve(event, {
filterSerializedResponseHeaders(name) {
return name === "content-range" ||
name === "x-supabase-api-version";
},
});
}; And it still creates the error message. J |
Ah, I didn't realize that. If supabase functions fetch the session using |
Hi, I am using quite the same stragtegy in SvelteKit to repopulate the user in the session received via supabase.auth.getSession() wtih the user from supabase.auth.getUser(). But as mentioned above already this does not solve e.g. the case of updateUser(). I consider the silecing of the logs rather to be a hack., but this is the option as well at the moment It is really a shame Supabase have not solved this issue for more the 3 or 4 month. Anyways here is my hooks.server.js with user object substitution as well as silincing of the logs. import { PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY } from '$env/static/public'
import { createServerClient } from '@supabase/ssr'
import addUserprofileToUser from './utils/addUserprofileToUser'
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: '/' })
)
},
},
})
if ('suppressGetSessionWarning' in event.locals.supabaseServerClient.auth) {
// @ts-expect-error - suppressGetSessionWarning is not part of the official API
event.locals.supabaseServerClient.auth.suppressGetSessionWarning = true;
} else {
console.warn(
'SupabaseAuthClient#suppressGetSessionWarning was removed. See https://github.com/supabase/auth-js/issues/888.',
);
}
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
await addUserprofileToUser(session, event.locals.supabaseServerClient, 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'
},
})
} |
Ping! Still an issue, why isn't this fixed? Just remove the warning and add it as a warning in the docs..... |
Thanks for raising the issue, the team is aware of this and actively working towards a fix. The team hopes to release asymmetric keys soon which will allow for session verification client side without a round trip to Supabase Auth. This should hopefully significantly alleviate or fix the issue. We are getting close to the date of release - please watch our changelog for further updates. Let us know if there are further questions. Thank you |
I'm doing this to suppress the annoying warnings. async function getSupabaseAuthSession(supabase: SupabaseClient) {
const [s, u] = await Promise.all([supabase.auth.getSession(), supabase.auth.getUser()])
if (!s.data?.session || !u.data?.user) return null
// @ts-expect-error
delete s.data.session.user
return { ...s.data.session, user: u.data.user } as Session
} |
any updates? Having this issue on Remix as well |
Bug report
Describe the bug
Maintainer of the nuxt/supabase module here.
We have a PR to migrate on the@supabase/ssr
package and we're still experiencing this issue with the latest released version including your PR.The module is now using the
@supabase/ssr
package and we're still experiencing this issue with the latest released version.I've removed all occurrences of getSession() in the module and I still have the warning.
Any help on this would be appreciate 🙏
To Reproduce
Clone the nuxt/supabase repository and follow the development readme to run the playground.
Notice the
Using the user object as returned from supabase.auth.getSession() or from some supabase.auth.onAuthStateChange() events could be insecure
warning.Expected behavior
Do not display this warning.
The text was updated successfully, but these errors were encountered: