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

Receiving warning when I'm not calling auth.getSession anywhere: Using 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() #755

Open
1 task
williamlmao opened this issue Mar 29, 2024 · 17 comments
Labels
bug Something isn't working

Comments

@williamlmao
Copy link

Bug report

  • [x ] 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

Despite not calling supabase.auth.getSession() anywhere in my code (as seen in screenshot), I get this warning nonstop in my server side terminal logs Using 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()

image

To Reproduce

import './globals.css'
import { Metadata } from 'next/types'
import { cookies } from 'next/headers'
import { createServerClient } from '@supabase/ssr'

export default async function RootLayout({
  children,
}: {
  children: React.ReactNode
}) {
  const cookieStore = cookies()

  const supabase = createServerClient(
    process.env.NEXT_PUBLIC_SUPABASE_URL!,
    process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!,
    {
      cookies: {
        get(name: string) {
          return cookieStore.get(name)?.value
        },
      },
    }
  )

  const user = await supabase.auth.getUser()

  return (
    <html
      lang="en"
    >
      <div className="">text</div>
    </html>
  )
}

Expected behavior

I expect it to not give me the error since I'm using the function anywhere. It would be nice if it told me where in my code it detected the issue.

System information

  • OS: macOs
    "@supabase/ssr": "^0.1.0",
    "@supabase/supabase-js": "2.39.1",
@williamlmao williamlmao added the bug Something isn't working label Mar 29, 2024
@sahmed007
Copy link

Was just about to make an issue about this as well. Getting the same warnings despite not using it.

@beelzick
Copy link

I have the same issue

@przemyslaw-paziewski
Copy link

Same here

@AlanKeown
Copy link

I'm getting the warning but I am using it according to the docs for SSR in Sveltekit.

@caanyp24
Copy link

I'm getting the same warning and i am not using supabase.auth.getSession()

@Cheveniko
Copy link

Same issue here, happened just after I updated the Nextjs and eslint-config-next version from 14.0.4 to 14.1.4.
Before that I wasn't getting the warning.

@jdgamble555
Copy link

This is a huge problem and a pain. I don't want to contact the Supabase server to see if a user is logged in. This will slow down my app!

This is a supabase core problem it seems, as I can't find the error message in this package.

J

@Pluriscient
Copy link

+1 to the problem, only call getSession in the hooks.server.ts with the code from the tutorial (adapted to prevent setting cookies after request has been returned)

  let called = false;
  /**
   * A convenience helper so we can just call await getSession() instead const { data: { session } } = await supabase.auth.getSession()
   */

  event.locals.getSession = async () => {
    called = true;
    const {
      data: { user },
    } = await event.locals.supabase.auth.getUser();
    let {
      data: { session },
    } = await event.locals.supabase.auth.getSession();

    // solving the case if the user was deleted from the database but the browser still has a cookie/loggedin user
    // +layout.server.js will delete the cookie if the session is null
    if (user == null) {
      session = null;
    }

    return session;
  };
  if (!called) {
    await event.locals.getSession();
  }

@jdgamble555
Copy link

https://github.com/supabase/auth-js/blob/92fefbd49f25e20793ca74d5b83142a1bb805a18/src/GoTrueClient.ts#L936 - Here is the culprit. I just created a new issue and linked it.

@Pluriscient - I'm not sure why you're doing that, but it doesn't work. You're just calling getSession() an extra time? What is the goal here?

J

@Pluriscient
Copy link

https://github.com/supabase/auth-js/blob/92fefbd49f25e20793ca74d5b83142a1bb805a18/src/GoTrueClient.ts#L936 - Here is the culprit. I just created a new issue and linked it.

@Pluriscient - I'm not sure why you're doing that, but it doesn't work. You're just calling getSession() an extra time? What is the goal here?

J

The console log actually asked to call getUser before getSession to suppress the warning. The if (!called) escape was included due to this issue

@jdgamble555
Copy link

Looks like that fix doesn't work

@Pluriscient
Copy link

Pluriscient commented Mar 31, 2024

Looks like that fix doesn't work

YMMV, but I haven't seen a crash due to setCookies since. This error popped up the day after when we ran a fresh npm install, though I don't see the ssr package being updated there.

@jdgamble555
Copy link

For my purposes this hack solves it. However, I am only using session to detect if there is a session, not get session data.

event.locals.getSession = async () => {

  const { data: { user } } = await event.locals.supabase.auth.getUser();

  if (user === null) {
    return null;
  }

  const session = { user };

  return session as Session;
};

My problem with this approach is that I need to check for a user session on my non logged in page. So I can give different results whether or not a user is logged in (likes, votes, etc on a post). This means even for non-logged-in users, I have to send an extra fetch request to Supabase in order to display my data. Perhaps it should only call getUser() if there is a session, and then verify it on the Supabase server.

This is problematic for sure.

J

@cowboycodr
Copy link

I have the same issue as well. Makes debugging in the console an absolute nightmare.

@hmnd
Copy link

hmnd commented Apr 9, 2024

Just chiming in with my vote in support of fixing this issue... here's what I said in another thread:

I understand the intention with this warning, but it's impossible to silence when I know I'm properly verifying with getUser. In my case, I need to use it with createSupabaseLoadClient from auth-helpers, and because it serializes the session as JSON, I get a warning every time I call that function or eg. spread the session into a new object, etc.

At the very least, could the warning only be displayed once per session or could we please have an option to disable it? I see that now it's gated behind a non-existent __suppressUserWarning key on session.

For reference the warning I'm getting is below. It's a different one from the one this discussion originally addresses:

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.

@ixxie
Copy link

ixxie commented May 5, 2024

As somebody just setting up a SvelteKit project with Supabase, this is quite confusing: copy-pasting code from your tutorial results in a warning about best practices! One more vote for a fix!

@j4w8n
Copy link

j4w8n commented May 9, 2024

Oops! I created an issue for this a couple of weeks after yours was created: supabase/auth-js#888

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