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

Still having getSession warning whenever _saveSession is called #912

Open
2 tasks done
larbish opened this issue May 17, 2024 · 16 comments
Open
2 tasks done

Still having getSession warning whenever _saveSession is called #912

larbish opened this issue May 17, 2024 · 16 comments
Labels
bug Something isn't working

Comments

@larbish
Copy link

larbish commented May 17, 2024

Bug report

  • I confirm this is a bug with Supabase, not with my own application.
  • I confirm I have searched the Docs, GitHub Discussions, and Discord.

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.

@larbish larbish added the bug Something isn't working label May 17, 2024
@j4w8n
Copy link
Contributor

j4w8n commented May 26, 2024

Have you heard back about this?

@regg00
Copy link

regg00 commented Jun 7, 2024

@larbish, you got any news about this issue?

@larbish
Copy link
Author

larbish commented Jun 18, 2024

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 @supabase/ssr package 🙏

@hf
Copy link
Contributor

hf commented Jun 19, 2024

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.

@j4w8n
Copy link
Contributor

j4w8n commented Jun 19, 2024

Looks like the code calls session.user a couple of times. And it json stringifies the session as well, which would also trigger the warning.

@scottandrew
Copy link

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.

@chbert
Copy link

chbert commented Jul 12, 2024

Same here, running:

"@supabase/ssr": "^0.4.0",
"@supabase/supabase-js": "^2.44.3",

Getting:

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.

Please let me know how I can support to resolve this!

@fnimick
Copy link

fnimick commented Jul 28, 2024

If you are calling getUser on requests and want to silence the warning as your session is guaranteed to be safe, see my approach in https://github.com/fnimick/sveltekit-supabase-auth-starter - I replace the user proxy object that triggers the warning with the user instance returned from getUser. I'm not sure what the nuxt equivalent is, but look at hooks.server.ts for my solution.

@jdgamble555
Copy link

I'm not calling or using session anywhere and it still gets called randomly.

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 getUser() right before that line, still doesn't fix it.

J

@j4w8n
Copy link
Contributor

j4w8n commented Jul 28, 2024

I'm not calling or using session anywhere and it still gets called randomly.

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 getUser() right before that line, still doesn't fix it.

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.

@jdgamble555
Copy link

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

@j4w8n
Copy link
Contributor

j4w8n commented Jul 29, 2024

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.

@fnimick
Copy link

fnimick commented Jul 29, 2024

Until a fix is out, the only way to eliminate warnings is replace the .user property on the session with one returned from .getUser, as I do in my example.

    // 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 };

@jdgamble555
Copy link

@fnimick - I don't even use the session variable. I believe most people don't need it.

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

@fnimick
Copy link

fnimick commented Jul 29, 2024

Ah, I didn't realize that. If supabase functions fetch the session using _useSession and then use it, the error will appear and there's no way to avoid it. My solution fixes it only for uses of session.user on the server side when that is safe (the user is fetched separately).

@kvetoslavnovak
Copy link

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'
      },
  })
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

9 participants