Skip to content

SameSite cookie support #1498

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

Merged
merged 1 commit into from
Jul 10, 2020
Merged

SameSite cookie support #1498

merged 1 commit into from
Jul 10, 2020

Conversation

chrishulbert
Copy link
Contributor

@chrishulbert chrishulbert commented Jul 3, 2020

Support SameSite cookie parameter

This has become an issue when developing locally, because most browsers now give warnings if you don't have SameSite set, and the warning states they will soon ban cookies that don't have SameSite or Secure set. So this would allow you to set SameSite so it'll work when developing locally over HTTP not HTTPS.

I'm not sure, but maybe it should default to 'strict' for local development so people don't get stung by this when browsers begin to enforce this rule; it seems like it'd be safer to leave this as 'default' for production so as to not break anyone's sites - however it might even better to default it to 'strict' for production for new sites too? Open to the maintainers thoughts on those issues.

Thanks all

# Conflicts:
#	revel.go
@chrishulbert
Copy link
Contributor Author

Documentation to match: revel/revel.github.io#196

@ptman
Copy link
Contributor

ptman commented Jul 9, 2020

I believe this is needed and an improvement. Apparently extra cookies can set their samesite property freely. What about i18n cookie?

@chrishulbert
Copy link
Contributor Author

Thanks @ptman for getting back about this.
I had a look for i18n cookies, and found that it reads the cookie in i18n.go > hasLocaleCookie, however I could not find where that cookie is set anywhere? Is this intended for the framework consumer to set that cookie or am i missing something?
Thanks again

@ptman
Copy link
Contributor

ptman commented Jul 10, 2020

Oh, my bad. Seems like revel doesn't set the locale cookie, only reads it if it's set.

@ptman
Copy link
Contributor

ptman commented Jul 10, 2020

what's up with the failing tests?

@notzippy
Copy link
Collaborator

what's up with the failing tests?

There is an issue with memcache an external project not finding the key occasionally during the tests.

@notzippy notzippy merged commit ff2da7e into revel:develop Jul 10, 2020
@chrishulbert chrishulbert deleted the feature/same-site-cookies branch July 13, 2020 23:22
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.

3 participants