nuxt-modules / supabase

Supabase module for Nuxt.
https://supabase.nuxtjs.org
MIT License
733 stars 129 forks source link

fix: ssr getSession warning #418

Closed th1m0 closed 1 month ago

th1m0 commented 1 month ago

Types of Changes

Description

This PR addresses a warning that occurs when calling supabase.auth.getSession() on the server:

 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.

To resolve this, the user property is removed from the session object within the server plugin. This prevents the user prop from being accessed and stops the warning from being logged.

An alternative approach, without deleting the user object, could be:

useSupabaseSession().value = session
  ? {
      access_token: session.access_token,
      expires_in: session.expires_in,
      token_type: session.token_type,
      refresh_token: session.refresh_token,
      provider_token: session.provider_token,
      provider_refresh_token: session.provider_refresh_token,
      expires_at: session.expires_at,
    }
  : null;

Checklist

netlify[bot] commented 1 month ago

Deploy request for n3-supabase pending review.

Visit the deploys page to approve it

Name Link
Latest commit 987ca63e2a2646ba03ffc2e5bd295ac46e3e34e0
th1m0 commented 1 month ago

After looking back on this, I thought this could be fixed a little bit better so I made a couple of changes and it will now also suppress the warning when using serverSupabaseSession as it will delete the user object there.

The server plugin now uses the utility functions serverSupabaseSession and serverSupabaseUser

negativems commented 1 month ago

Any update? when will be merged?

th1m0 commented 1 month ago

Any update? when will be merged?

Depends on when @larbish has the time to review and merge it. This change shouldn't be high prio tho as it will only suppress a warning and prevent users from accessing session.user on the server which would be unsafe.

When client side rendering however it wouldn't be "unsafe" but since we have useSupabaseUser you should just always use that to access the user.

For now you can just ignore the warning.

BritLuey commented 1 week ago

Thank you so much for this PR, this warning was driving me insane!