Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Security/Http] Check tokens before loading users from providers#49078
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
f00b9ed toec78c9bComparenicolas-grekas commentedJan 23, 2023
(deps=high failures need a merge up to be green). |
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Security/Core/Signature/SignatureHasher.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Security/Http/RememberMe/PersistentRememberMeHandler.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Security/Http/RememberMe/SignatureRememberMeHandler.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
ec78c9b to889d739Compare
nicolas-grekas left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Comments addressed@stof, thanks for the review. New round unlocked :)
…legacy tokens (nicolas-grekas)This PR was merged into the 5.4 branch.Discussion----------[Security/Http] Fix compat of persistent remember-me with legacy tokens| Q | A| ------------- | ---| Branch? | 5.4| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets |Fix#49100| License | MIT| Doc PR | -In#49078, we changed the format of remember-me tokens, effectively invalidating them all.While the invalidation is intentional for signature-based remember-me handlers, persistent remember-me handlers could accept both legacy and updated tokens.This PR fixes compat with legacy tokens for persistent remember-me handlers.Commits-------538d660 [Security/Http] Fix compat of persistent remember-me with legacy tokens
Remember me cookies and login link handler tokens contain an expiry but we check this expiry only after we've loaded a user from a provider. This can create unneeded load on the provider. Note that the now legacy security subsystem was free from this issue so this PR is fixing a regression.
For persistent tokens, I've removed any logic to sign them in
PersistentRememberMeHandlerbecause we never validate the signature, so it's just useless.