Add SameSite to DiscardingCookie

[mkurz]

Fixes #10122

See

• can't delete cookie with clearCookie: should add SameSite param symfony/symfony#36107
  • [Http Foundation] Fix clear cookie samesite symfony/symfony#36173
• https://web.dev/samesite-cookie-recipes/
• https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite
• https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie
• https://www.thinktecture.com/en/identity/samesite/how-to-delete-samesite-cookies/

[gmethvin]

Do we actually need pass the sameSite attribute to DiscardingCookie for deleting a cookie?

We probably want to use the broadest possible SameSite attribute for deleting, i.e.:

def toCookie = {
  // `SameSite=None` requires `Secure`
  val sameSite = if (secure) SameSite.None else SameSite.Lax
  Cookie(name, "", Some(Cookie.DiscardedMaxAge), path, domain, secure, false, Some(sameSite))
}

This will potentially overwrite the existing SameSite attribute, but that should be fine, since the cookie is being deleted anyway.

[mkurz]

@gmethvin Do you think it's safe to send SameSite=None per default for discarding a cookie (assuming secure = true, which is very likely nowadays)?
I have two serious concerns (both described here):
First: There was this infamous Safari bug, which caused SameSite=None to be treated like SameSite=Strict.
Second: Older Chromium versions can cause problems, given that such old version may still be used in other Chromium derived browsers.
For the Safari bug I don't see a problem however, because a cookie which does not get send cross domain, it should not matter if the browser handles the cookie as SameSite None or Strict, it still should discard the cookie in both cases (for cross domain it probably would not work, but it doesn't work now as well anyway). I am aware this Safari bug was fixed, however I guess there are millions of people out there that are stuck with iOS 12 and macOS 10.14
However for the Chromium derived browsers the problem would be that "These Chrome versions will reject a cookie with SameSite=None" which of course is much worse! Not sure how many such browsers out there, however for them the Discard cookie would suddenly be completely broken.

Another concern I have: Could there be cases where setting SameSite=None would allow an attacker to delete a cookie in situations were it would not have been possible when using Lax? I mean, as a framework user, I set Samesite=Lax in application.conf for the session cookie, but now the framework would overrule that setting when discarding the cookie, setting it to None... Imagine a very stupid case (based on this issue): You have a page that, when accessed, removes a cookie by sending the Set-Cookie header. Now when you access this side via an iframe, currently the Set-Cookie header, without sending a Samesite attribute, would default to Lax (in recent browser versions), meaning the Set-Cookie would be ignored. By setting SameSite to None by default for discarding cookies, suddenly the Set-Cookie would work now...
I hope you understand my example and I know it's a stupid one, my point is that maybe we are "opening" the discard cookie to be executed in cases where it shouldn't. I can't think of a other case right now but maybe there are ones...

Actually the more I think about this I think for the discard cookie we should just use the same SameSite attribute like when the cookie was created... I think that makes the most sense: In the end the user has control which SameSite attribute will be send (for session/flash cookies that would be in application.conf).

[gmethvin]

To be honest I haven't thought through all the potential browser bugs and special cases, so I guess that's a good point.

Actually the more I think about this I think for the discard cookie we should just use the same SameSite attribute like when the cookie was created... I think that makes the most sense: In the end the user has control which SameSite attribute will be send (for session/flash cookies that would be in application.conf).

Yes, this would probably be the best solution. If we know the value of the original cookie that would be the most intuitive API. For session and flash cookies of course this would be easy, but I think for other cookies we would need to retrieve the original cookie from the request.

[mkurz]

but I think for other cookies we would need to retrieve the original cookie from the request.

I don't think that is something the framework should care about. The framework user created the cookie, so the user should just make sure to set the correct SameSite attribute when discarding the cookie.

[mkurz]

@gmethvin Actually this pull request is ready now.

[gmethvin]

looks fine to me; definitely an improvement. It would be nice if there was a way to automatically pass the right SameSite for the Java API, but the change as-is is consistent with the way it currently works so I think it's fine in that sense.

[gmethvin]

I guess this means you'll have to switch to this API to fix the samesite issue for custom cookies?

[mkurz]

Yeah, you have to use this new method that takes the Cookie.SameSite param.