Add laxSameSiteSessions and strictSameSiteSessions #1226
Conversation
| @@ -21,3 +21,4 @@ extra-deps: | |||
| - yaml-0.8.17 | |||
| - nonce-1.0.2 | |||
| - persistent-2.5 | |||
| - cookie-0.4.2 | |||
There was a problem hiding this comment.
The current cookie package constraint is >= 0.4.1, but the SameSite options were added in 0.4.2. Can you bump the cookie constraint in the cabal file to handle this?
There was a problem hiding this comment.
Done, but without this update the resolver fails with:
Failure when adding dependencies:
cookie: needed (>=0.4.2 && <0.5), 0.4.1.6 found (latest applicable is 0.4.2)
needed for package: yesod-core-1.4.20.2
| @@ -366,6 +366,22 @@ sslOnlySessions = (fmap . fmap) secureSessionCookies | |||
| setSecureBit cookie = cookie { setCookieSecure = True } | |||
| secureSessionCookies = customizeSessionCookies setSecureBit | |||
|
|
|||
| -- | Helps defend against CSRF attacks by setting the SameSite attribute on | |||
| -- session cookies to "Lax". | |||
There was a problem hiding this comment.
I think this documentation would be more helpful if it briefly explained what this attribute does, and/or linked to an external resource with a good explanation, similar to sslOnlySessions's documentation. That would help users who are trying to figure out how Yesod protects against CSRF attacks, but aren't familiar with this specific method of prevention. What do you think of something like this?
-- | Helps defend against CSRF attacks by setting the SameSite attribute on
-- session cookies to "Lax". See the <https://tools.ietf.org/html/draft-west-first-party-cookies IETF Draft for details>.
or more detailed:
-- | Helps defend against CSRF attacks by setting the SameSite attribute on
-- session cookies to "Lax". This will cause browsers to not include your session cookie
-- when a write request is made from www.attacker.com to your site. HTTP requests using
-- @GET@, @HEAD@, @OPTIONS@ or @TRACE@ aren't affected by this setting.
--
-- See the <https://tools.ietf.org/html/draft-west-first-party-cookies IETF Draft for Details>.
--
-- /Warning:/ SameSession cookies were introduced mid-2016, so many browsers won't support them.
-- You should use SameSite session cookies in combination with normal CSRF prevention methods.
laxSameSiteSessions :: ...
-- | Helps defend against CSRF attacks by setting the SameSite attribute on
-- session cookies to "Strict". The "Strict" option functions the same as the "Lax" option,
-- but also removes cookies for @GET@, @HEAD@, @OPTIONS@ and @TRACE@ HTTP methods.
strictSameSiteSessions :: ...
Caveat: I only learned about the SameSite cookie attribute when you opened this PR, so you probably know more about what this documentation should say
There was a problem hiding this comment.
I see what you are saying. As the spec is still evolving it's best if people do up to date research when considering whether or not to use this option. I'll think about how to clearly document that.
There was a problem hiding this comment.
As the spec is still evolving
Hm, ok. Here are some key things that might be worth covering:
- Warn that browser support is recent/forthcoming. Otherwise, people might use SameSite cookies and they'll be confused why it isn't working for them, or it might work for them and they won't realize many of their users aren't protected. A link to caniuse.com would be great, but they don't support it yet, unfortunately.
- Warn that the yesod API may need to change, since the spec is still in flux, if you think that might need to happen
- Suggest checking the latest draft of the spec. Is this URL https://tools.ietf.org/html/draft-west-first-party-cookies sufficient? It takes you the latest version of the spec—not sure if they intend to move it from that site later.
There was a problem hiding this comment.
thought about this, and I think we should have:
- Link to https://datatracker.ietf.org/doc/draft-west-first-party-cookies/
- Mention that the cookie option is new and changeable.
- Explain briefly what the (current) difference is between Lax and Strict.
| strictSameSiteSessions = sameSiteSession sameSiteStrict | ||
|
|
||
| sameSiteSession :: SameSiteOption -> IO (Maybe SessionBackend) -> IO (Maybe SessionBackend) | ||
| sameSiteSession s = (fmap . fmap) secureSessionCookies |
There was a problem hiding this comment.
I'm a little concerned that this helper is now part of the API (this module does not have an export list). @MaxGabriel do you think I should factor this back into laxSameSiteSessions and strictSameSiteSessions?
There was a problem hiding this comment.
Yesod.Core.Class.Yesod isn't listed under exposed-modules in the cabal file, so I don't think sameSiteSession would become public API.
|
This is good to merge I think |
|
Thanks! Sorry for the delayed merge |
Adds the ability to mark the Yesod session with SameSite options.