-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpFoundation][FrameworkBundle] fix support for samesite in session cookies #35605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
a46477d
to
0a530a8
Compare
0a530a8
to
f46e6cb
Compare
fabpot
approved these changes
Feb 7, 2020
Thank you @nicolas-grekas. |
fabpot
added a commit
that referenced
this pull request
Feb 7, 2020
… in session cookies (fabpot) This PR was merged into the 3.4 branch. Discussion ---------- [HttpFoundation][FrameworkBundle] fix support for samesite in session cookies | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #35520 | License | MIT | Doc PR | - This PR cherry-picks #28168 on 3.4, with a rationale given by @ConneXNL in #35520 (comment): > I hope I am wrong but I see the impact of not making any changes to Symfony 3.4 will have a tons of sites break if we cannot set the cookie's samesite setting (in the framework session and remember me) before Chrome pushes this update. > > Very soon all existing cookies are no longer going to work with cross-domains if you do not specify 'None' for the cookie_samesite. All external APIs that use cookies and are running SF 3.4 will break and devs will have no quick solution to fix their auth process. > > If you are using PHP 7.4, yes you can most likely use ini_set to workaround this issue. > > However, ini_set('cookie_samesite') does not work in PHP Version <= 7.2. I am not even sure PHP 7.3 supports the value 'None' as php.watch/articles/PHP-Samesite-cookies says it has support for 'Lax' and 'Scrict'. > > This effectively means SF 3.4 on PHP 7.2 (or PHP 7.3) is no longer supported for cross domain APIs with cookies. People would have to either update PHP to 7.4 (if they even can?) or go to Symfony 4 (with a dead live site is going to be a complete disaster). > > Since the impact of the change that chrome is about to roll out is so fundamentally changing our way to set cookies, I consider configuring samesite configuration in the framework an absolute requirement, not a feature, especially since SF 3.4 is still supported. > > What am i missing? > > Note: SF3 HTTPFoundation already supports the new cookie settings, it's just the framework that doesn't support it. Our BC policy embeds the promise that one should be able to keep the same app on a newest infrastructure (eg that's why supporting a PHP version is a bug fix). I think we can consider this for browsers here also. WDYT? Commits ------- f46e6cb [HttpFoundation][FrameworkBundle] fix support for samesite in session cookies
This was referenced Feb 29, 2020
Merged
Merged
Merged
nicolas-grekas
added a commit
that referenced
this pull request
Mar 23, 2020
…kie flag (dunglas) This PR was merged into the 3.4 branch. Discussion ---------- [Security/Http] Remember me: allow to set the samesite cookie flag | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Similar to #35605, since Chrome 80 is going to require the `samesite` attribute. This is a cherry-pick of #27976 Commits ------- f0ceb73 [Security] Remember me: allow to set the samesite cookie flag
11 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR cherry-picks #28168 on 3.4, with a rationale given by @ConneXNL in #35520 (comment):
Our BC policy embeds the promise that one should be able to keep the same app on a newest infrastructure (eg that's why supporting a PHP version is a bug fix). I think we can consider this for browsers here also. WDYT?