Skip to content
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

[Http Foundation] Fix clear cookie samesite #36173

Merged
merged 1 commit into from Mar 23, 2020
Merged

Conversation

@guillbdx
Copy link
Contributor

guillbdx commented Mar 23, 2020

Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #36107
License MIT

With Chrome Update 80, Cookies are required to be secure and samesite=none for cross site requests. However they are defaulted to samesite=lax if the samesite attribute is not set. In other words: developer has to explicitely opt-in for samesite=none in the case of a cross site request.

More details: https://chromestatus.com/feature/5088147346030592

We add the samesite argument to clearCookie method to allow developer to explicitely set this value.

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Mar 23, 2020
@guillbdx guillbdx closed this Mar 23, 2020
@guillbdx guillbdx reopened this Mar 23, 2020
@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Mar 23, 2020

Thank you @guillbdx.

@nicolas-grekas nicolas-grekas merged commit b4ec8b9 into symfony:3.4 Mar 23, 2020
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details
This was referenced Mar 27, 2020
@fabpot fabpot mentioned this pull request Mar 30, 2020
fabpot added a commit that referenced this pull request Mar 30, 2020
… delete_cookies (wouterj)

This PR was merged into the 3.4 branch.

Discussion
----------

[Security/Http] Allow setting cookie security settings for delete_cookies

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #36243 (comment)
| License       | MIT
| Doc PR        | tbd

Similar to #36173 and #36175. This is needed for Chrome 80 compatibility.

My only question is whether we should introduce these specific settings, or somehow fetch them from `framework.session`?

Commits
-------

a696d1f [Security/Http] Allow setting cookie security settings for delete_cookies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.