Allow opting out of the SameSite cookie attribute when setting a cookie
#45501
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghiculescu
force-pushed
the
same-site-false
branch
from
f5994cd
to
e636b40
Compare
| end | ||
|
|
||
| def test_setting_cookie_with_specific_same_site_strict | ||
| @request.env["action_dispatch.cookies_same_site_protection"] = proc { :lax } |
There was a problem hiding this comment.
This is already set in setup, so we could possibly omit. I do see an argument for being explicit here, though.
There was a problem hiding this comment.
I think it is better to be explicit since all the tests around it have the same pattern.
ghiculescu
force-pushed
the
same-site-false
branch
from
5f263c0
to
1379a50
Compare
…okie. Since rails@7ccaa12 it's not been possible to not include `SameSite` on your cookies. `SameSite` is recommended, but it's not a required field, and you should be able to opt out of it. This PR introduces that ability: you can opt out of `SameSite` by passing `same_site: false`. ```ruby cookies[:foo] = { value: "bar", same_site: false } ``` Previously, this incorrectly set the `SameSite` attribute to the value of the `cookies_same_site_protection` setting. rails#44934 added docs saying that you could pass `nil` as a value, but that also would fall back to the default (`:lax`).
|
Good call @jonathanhefner. The API is much simpler now 😍 |
ghiculescu
force-pushed
the
same-site-false
branch
from
1379a50
to
d29e755
Compare
jonathanhefner
approved these changes
Jul 5, 2022
|
Thank you, @ghiculescu! 🙌 |
jonathanhefner
added a commit
that referenced
this pull request
Jul 24, 2022
Rails 7.0 does not support opting out of the `SameSite` attribute by specifying `same_site: nil`. (However, #45501 will add support for this in the next Rails release.)
jonathanhefner
added a commit
to jonathanhefner/rails
that referenced
this pull request
Jul 28, 2022
Follow-up to rails#45501. The Rack base class that `CookieStore` inherits from [always sets `:same_site`][1]. Thus, `options.key?(:same_site)` always returns true for session cookies, preventing a default value from being set. It would be possible to change Rack to conditionally set `:same_site`, but, from Rack's perspective, it has no reason to not set `:same_site`, because it treats a `nil` value the same as no value. Therefore, this commit specifies a default `:same_site` in `CookieStore`, which simply defers to `request.cookies_same_site_protection` as `CookieJar` does. Fixes rails#45681. [1]: https://github.com/rack/rack/blob/2.2.4/lib/rack/session/abstract/id.rb#L398-L402
|
Glad you were able to draw attention to your PR. I didn't on my own patch long ago :'( #42962. Yours is better anyway. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since #37974 it's not been possible to not include
SameSiteon your cookies.SameSiteis recommended, but it's not a required field, and you should be able to opt out of it.This PR introduces that ability: you can opt out of
SameSiteby passingsame_site: nil.Previously, this incorrectly set the
SameSiteattribute to the value of thecookies_same_site_protectionsetting. #44934 added docs saying that you could passnilas a value, but that also would fall back to the default (:lax).