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

Change SameSite to Lax during Development in app.lua #284

Merged
merged 5 commits into from
Jul 27, 2021

Conversation

ochan1
Copy link
Contributor

@ochan1 ochan1 commented Apr 27, 2021

Related Issue: #277

Confirmed that setting the SameCookie attribute to "Lax" fixes the issue for development

"None" works in the deployed website (for some reason)

Tested on Chrome and Opera (both Chromium browsers)

@cycomachead
Copy link
Member

Thanks! This is good to know it fixes the development issues. Not sure why I haven't seen them personally.

However, I think we need SameSite=None on production still. Every time I re-read about this, it gets more complicated.
Would you be able to upload the secure variable to be SameSite=None; Secure; when not development and just '' when in development mode. Leaving off SameSite in development is the same as setting it to Lax.

@ochan1 ochan1 changed the title Change SameSite to Lax in app.lua Change SameSite to Lax during Development in app.lua Apr 28, 2021
@ochan1
Copy link
Contributor Author

ochan1 commented Apr 28, 2021

Changes made as described

Dedicated "if" statement for Development to set the proper setting for secure and sameSite, default to production values

@bromagosa
Copy link
Collaborator

Does this lgtty, @cycomachead? If so let's go ahead and pull it.

@bromagosa
Copy link
Collaborator

@cycomachead ping? :)

@cycomachead
Copy link
Member

Whoops, sorry I missed this. Looks good, thanks Oscar!!

@cycomachead cycomachead merged commit 3ac5c48 into snap-cloud:master Jul 27, 2021
@ochan1 ochan1 deleted the cookie_samesite_lax_chromium branch July 27, 2021 05:58
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

3 participants