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

Update user-create-mutation.ts to not omit Cookie Data #18156

Merged
merged 2 commits into from
Nov 13, 2023

Conversation

activenode
Copy link
Contributor

@activenode activenode commented Oct 12, 2023

Avoid stripping away cookie data and hence avoid having problems with Cookie Based Authentication Proxies

What kind of change does this PR introduce?

I'd assume it's a Bug as it unnecessarily strips away Cookies which other requests don't.

What is the current behavior?

https://github.com/supabase/supabase/blame/072222523afe612d575c2ff7e73e514692f545af/studio/data/auth/user-create-mutation.ts#L50

Currently credentials are stripped with credentials: 'omit'
Ref: https://developer.mozilla.org/en-US/docs/Web/API/Request/credentials

That means that in any Proxy Setup which uses Authentication the Auth system cannot read the credentials and the request fails for no good reason.

What is the new behavior?

Credentials are not stripped, the default behaviour of passing credentials to the same site, which for obvious reasons is safe, is the new default.

Additional context

Was actually reported by one of the users (colin) in the YouTube video here https://www.youtube.com/watch?v=wyUr_U6Cma4

Avoid stripping away cookie data and hence avoid having problems with Cookie Based Authentication Proxies
@activenode activenode requested a review from a team as a code owner October 12, 2023 18:14
@vercel
Copy link

vercel bot commented Oct 12, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 13, 2023 5:01am
studio-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 13, 2023 5:01am
zone-www-dot-com ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 13, 2023 5:01am
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
studio ⬜️ Ignored (Inspect) Visit Preview Nov 13, 2023 5:01am
studio-self-hosted ⬜️ Ignored (Inspect) Visit Preview Nov 13, 2023 5:01am
ui-storybook ⬜️ Ignored (Inspect) Visit Preview Nov 13, 2023 5:01am

@vercel
Copy link

vercel bot commented Oct 12, 2023

@activenode is attempting to deploy a commit to the Supabase Team on Vercel.

A member of the Team first needs to authorize it.

@activenode
Copy link
Contributor Author

Can anyone have a look on this one? It's a really simple fix. @burggraf do you know who is responsible for this type of issue?

Copy link
Member

@alaister alaister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @activenode,

My apologies for the delay here – thanks for the PR!

@alaister alaister enabled auto-merge (squash) November 13, 2023 04:40
@alaister alaister merged commit eaf185c into supabase:master Nov 13, 2023
13 checks passed
encima pushed a commit that referenced this pull request Nov 13, 2023
Avoid stripping away cookie data and hence avoid having problems with Cookie Based Authentication Proxies

Co-authored-by: Alaister Young <[email protected]>
encima pushed a commit to faizanraso/supabase that referenced this pull request Jan 8, 2024
Avoid stripping away cookie data and hence avoid having problems with Cookie Based Authentication Proxies

Co-authored-by: Alaister Young <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants