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

[Security] Fix clearing remember-me cookie after deauthentication #34671

Merged
merged 1 commit into from Nov 30, 2019

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Nov 28, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #26379
License MIT
Doc PR -

If you are using the remember_me listener and the refreshed user is deauthenticated, you are still logged in because the remember-me cookie does not get cleared.
This fixes it.

@chalasr chalasr force-pushed the rememberme-logout branch 4 times, most recently from 18ca151 to 1f57f19 Compare November 29, 2019 22:30
@fabpot
Copy link
Member

fabpot commented Nov 30, 2019

Thank you @chalasr.

fabpot added a commit that referenced this pull request Nov 30, 2019
…ication (chalasr)

This PR was merged into the 3.4 branch.

Discussion
----------

[Security] Fix clearing remember-me cookie after deauthentication

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #26379
| License       | MIT
| Doc PR        | -

If you are using the `remember_me` listener and the refreshed user is deauthenticated, you are still logged in because the remember-me cookie does not get cleared.
This fixes it.

Commits
-------

d625a73 [Security] Fix clearing remember-me cookie after deauthentication
@fabpot fabpot merged commit d625a73 into symfony:3.4 Nov 30, 2019
@chalasr chalasr deleted the rememberme-logout branch November 30, 2019 13:22
@@ -537,6 +538,9 @@ private function createAuthenticationListeners($container, $id, $firewall, &$aut
} elseif ('remember_me' === $key) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Careful when merging up: this condition also includes anonymous === $key as of 4.4, it needs to be split so that this applies only to remember_me.

always_remember_me: true
secret: key
anonymous: ~
logout_on_user_change: true
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be removed starting from 4.3

@fabpot fabpot mentioned this pull request Dec 1, 2019
This was referenced Dec 1, 2019
@gondo
Copy link
Contributor

gondo commented Dec 2, 2019

this broke my application. can't login anymore, I am being redirected to login page even with correct credentials. downgrading to SF 3.4.35 (from 3.4.36) fixed the problem.
SF Profile doesn't show any error msg, dev log file is empty.

@chalasr
Copy link
Member Author

chalasr commented Dec 2, 2019

@gondo please open an issue with enough information to reproduce

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants