supabase / auth-helpers

A collection of framework specific Auth utilities for working with Supabase.
https://supabase.github.io/auth-helpers/
MIT License
893 stars 240 forks source link

cookieOption field "name" is not correctly working #717

Open cp-20 opened 6 months ago

cp-20 commented 6 months ago

Bug report

Describe the bug

I used createPagesBrowserClient for my application and set cookieOption to the below but stored cookie didn't change its name.

{
    name: 'some-cookie-name',
    secure: true,
    sameSite: "Lax",
    domain: "",
    path: "/",
}

I suspect that https://github.com/supabase/auth-helpers/commit/f7e5c2dc01248868e763b93a12c0539eb4bcc615 makes this bug. Base cookie name passed to createClient of @supabase/supabase-js comes from option.auth.storageKey before this commit, but storageKey field is removed and nothing other is changed after it. Please consider reverting it or take another solution.

fnimick commented 6 months ago

This is blocking a migration from @supabase/auth-helpers-sveltekit (where the name option works) to ssr (where the name option no longer works). I can confirm that the name option has no effect on the cookie used for auth data.

kangmingtay commented 5 months ago

@cp-20 @fnimick we fixed this in the supabase/ssr package in #730 - i'll take a look at the other packages to see if the same bug is present there too

binury commented 4 months ago

@cp-20 @fnimick we fixed this in the supabase/ssr package in #730 - i'll take a look at the other packages to see if the same bug is present there too

I could be mistaken but #730 change applies to the serverClient but not browserClients which will still use sb-${new URL(this.authUrl).hostname.split('.')[0]}-auth-token. Without the change you made to the defaults on serverClient, it's still only going to be making "anon" requests and looking for different session.

Apparently this did exist at one point https://github.com/supabase/auth-helpers/pull/677 but was reverted for unclear reason(s).

jorgebaralt commented 4 months ago

Any reason it got reverted? I really need this

binury commented 4 months ago

Any reason it got reverted? I really need this

as a workaround, you need to manually set the storageKey and the cookie.name to the same value. or, don't use the beta ssr auth-helper

jorgebaralt commented 4 months ago

Any reason it got reverted? I really need this

as a workaround, you need to manually set the storageKey and the cookie.name to the same value. or, don't use the beta ssr auth-helper

Thanks for the help.

what do you mean beta? afaik is the recommended way now? https://supabase.com/docs/guides/auth/server-side/migrating-to-ssr-from-auth-helpers

Also, about setting the storageKey, you mean patch the library myself? I do not see any storageKey param available

kangmingtay commented 4 months ago

hey @binury, thanks for catching that - i've made a fix for it in #746

pseacrest commented 3 months ago

I can confirm that the issue lies within Supabase and not within my own application. I have thoroughly searched through the documentation, GitHub Discussions, and Discord for any relevant information.

Description of the Bug:

In my application, I utilized createPagesBrowserClient and set the cookieOption as shown below. However, despite this configuration, the stored cookie did not change its name:

{
    name: 'some-cookie-name',
    secure: true,
    sameSite: "Lax",
    domain: "",
    path: "/",
}

I suspect that the bug may be related to commit hash f7e5c2d. Prior to this commit, the base cookie name passed to createClient of @supabase/supabase-js was derived from option.auth.storageKey. However, after this commit, the storageKey field was removed and no other changes were made. I suggest considering a revert of this commit or exploring alternative solutions.