Skip to content

Conversation

alexspeller
Copy link
Contributor

Recently, rack added support for SameSite=None cookies

However there is currently no way to set these cookies using the Rack::Session::Cookie
middleware without monkeypatching.

This pull request allows setting the SameSite value either by direct, literal
passthrough to the add_cookie_to_header method, or by passing a callable.

The callable option is required because some browsers are incompatible with
some values of the header, so it needs to be different per-browser.

Static usage:

use Rack::Session::Cookie, secret: 'supersecret', same_site: :none

Dynamic usage:

use Rack::Session::Cookie,
  secret: 'supersecret',
  same_site: lambda { |req, res| MyUtilClass.correct_value_for_samesite(req.user_agent) }

Copy link
Contributor

@jeremyevans jeremyevans left a comment

Choose a reason for hiding this comment

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

Other than one issue in the specs mentioned in an earlier note, I think this looks good and I plan to merge it unless another core team member objects.

…ack::Session::Cookie middleware

Recently, rack added support for SameSite=None cookies: rack#1358

However there is currently no way to set these cookies using the Rack::Session::Cookie
middleware without monkeypatching.

This pull request allows setting the SameSite value either by direct, literal
passthrough to the add_cookie_to_header method, or by passing a callable.

The callable option is required because some browsers are incompatible with
some values of the header, so it needs to be [different per-browser](https://www.chromium.org/updates/same-site/incompatible-clients).

Static usage:

```ruby
use Rack::Session::Cookie, secret: 'supersecret', same_site: :none
```

Dynamic usage:

```ruby
use Rack::Session::Cookie,
  secret: 'supersecret',
  same_site: lambda { |req, res| SameSite.value(req.user_agent) }
```
@alexspeller alexspeller force-pushed the allow-same-site-in-session-options branch from 4868ea4 to 8fee2fa Compare February 1, 2020 14:24
Copy link
Contributor

@jeremyevans jeremyevans left a comment

Choose a reason for hiding this comment

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

Looks good. I'll wait for a few days to see if another core team member has any feedback. Assuming none, I'll merge.

@jeremyevans jeremyevans merged commit dbd33f7 into rack:master Feb 4, 2020
kamipo added a commit to kamipo/rack that referenced this pull request Feb 10, 2020
ioquatix pushed a commit that referenced this pull request Feb 10, 2020
ioquatix pushed a commit that referenced this pull request Feb 10, 2020
bf4 referenced this pull request in rails/rails Jan 25, 2021
…e_site_protection

Add documentation of Proc usage for SameSite property to configuring.md

Address PR comments

Co-authored-by: Adrianna Chang <adrianna.chang@shopify.com>
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.

2 participants