Skip to content

Allow passing through same_site option to session cookie when using Rack::Session::Cookie middleware#1543

Merged
jeremyevans merged 1 commit into
rack:masterfrom
alexspeller:allow-same-site-in-session-options
Feb 4, 2020
Merged

Allow passing through same_site option to session cookie when using Rack::Session::Cookie middleware#1543
jeremyevans merged 1 commit into
rack:masterfrom
alexspeller:allow-same-site-in-session-options

Conversation

@alexspeller

Copy link
Copy Markdown
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) }

@jeremyevans jeremyevans left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread test/spec_session_cookie.rb Outdated
…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

@jeremyevans jeremyevans left a comment

Copy link
Copy Markdown
Contributor

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