8000 [Security] Fix persistent remember me being only usable once by Seldaek · Pull Request #42162 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Security] Fix persistent remember me being only usable once #42162

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

Closed
wants to merge 1 commit into from

Conversation

Seldaek
Copy link
Member
@Seldaek Seldaek commented Jul 16, 2021
Q A
Branch? 5.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets
License MIT
Doc PR

Quite the head-scratching debugging session to find this one 😄

It appears the new persistent remember me was broken in this way from the very first iteration https://github.com/symfony/symfony/blame/a942b5f6849810d45aab163c40381a16d5e8dbd9~1/src/Symfony/Component/Security/Http/RememberMe/PersistentRememberMeHandler.php#L77-L80

Essentially what happens is the first time (on login) you get a cookie with a hashed token, which is also stored in token provider. Then when session drops off, the handler recreates a new cookie but with the new un-hashed token. However the new hashed token is persisted, so when the session drops off again your cookie cannot be verified anymore, and you get logged out.

The fix is simply to correctly create new cookies with the hashed token.

Please be aware that if trying to repro this, due to line 91 clearing the session cookie isn't enough, you need to also wait a minute to really trigger a new token to be created.

@chalasr
Copy link
Member
chalasr commented Jul 16, 2021

Duplicate of #41897, right?

@Seldaek
Copy link
Member Author
Seldaek commented Jul 16, 2021

Oh yes, absolutely.. Except that perhaps the other PR did not highlight the level of urgency and bugginess :)

I'm happy to have this one closed, but I'd really hope this can be merged soon as it renders the remember-me feature practically unusable for us.

@Seldaek Seldaek closed this Jul 16, 2021
@Seldaek Seldaek deleted the patch-13 branch July 16, 2021 21:56
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.

3 participants
0