play1
play1 copied to clipboard
GH-1328: Added SameSite support for session cookie.
Pull Request Checklist
- [x] Have you read How to write the perfect pull request?
- [x] Have you read through the contributor guidelines?
- [x] Have you referenced any issues you're fixing using commit message keywords?
- [x] Have you updated the documentation?
- [x] Have you added tests for any changed functionality?
Helpful things
Fixes
Fixes #1328
Purpose
What does this PR do? This PR add support for SameSite attribute on the session cookie (https://owasp.org/www-community/SameSite)
Background Context
Why did you take this approach? At first, I created PlayCookie.java which extended Cookie.java, and DefaultPlayCookie.java extending DefaultCookie and implementing PlayCookie because Netty does not support SameSite attribute. I realized that for this to work, I had to implement PlayServerCookieEncoder to override encode methods in ServerCookieEncoder and for the encoder to work I had to copy CookieUtil from Netty in to the Play source code as it was package private. It worked, but it was a lot of code copy and duplication.
I refactored the code and ended up with this solution. Instead of implement a new version of ServerCookieEncoder, I call the original encoder (in Playhandler) from Netty and add SameSite at the end of the encoded string if the SameSite attribute is defined.
The reason for adding SameSite at the end of the encoded String and not implementing new classes is so it will be easier to adopt if we migrate to a new version of Netty with SameSite support in the future.
The SameSite attribute can be defined in "application.conf" with "application.session.sameSite" Can ble not defined, blank, "None", "lax", "strict". Not sure if casing is browser independent. "Lax" and "lax" resulted in "Lax" in Chrome, but "none" was only registered with capital N ("None").
References
Are there any relevant issues / PRs / mailing lists discussions? Not that IO
Updated PR with requested changes @asolntsev
Updated branch with master. Fixed merge conflict and updated failing testcases.
I have updated PR with changes proposed by @asolntsev. Changed "SAMESITE" enum to "SameSite"