Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[SecurityBundle] Renamefirewalls.logout.csrf_token_generator tofirewalls.logout.csrf_token_manager#48387
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
carsonbot commentedDec 5, 2022
Hey! I think@TimoBakx has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
90e4d48 to5f56da3Compare…irewalls.logout.csrf_token_manager`
5f56da3 to0a0a98aComparefabpot commentedDec 22, 2022
Thank you@MatTheCat. |
| * Implementing the`ConstraintViolationInterface` without implementing the`getConstraint()` method is deprecated | ||
| * Implementing the`ConstraintViolationInterface` without implementing the`getConstraint()` method is deprecated | ||
| * Deprecate the`security.firewalls.logout.csrf_token_generator` config option, use`security.firewalls.logout.csrf_token_manager` instead |
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.
this should not be in theValidator section
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.
fixed in377982f
…csrf_token_manager` (MatTheCat)This PR was squashed before being merged into the 6.3 branch.Discussion----------[Security] Rename logout’s `csrf_token_generator` to `csrf_token_manager`Follow-up ofsymfony/symfony#48387Should I still mention the deprecated `csrf_token_generator` in the documentation? If yes, how?Commits-------ced6562 [Security] Rename logout’s `csrf_token_generator` to `csrf_token_manager`
… to XSD (HeahDude)This PR was merged into the 6.3 branch.Discussion----------[SecurityBundle] Add `firewalls.logout.csrf_token_manager` to XSD| Q | A| ------------- | ---| Branch? | 6.3| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets | ~| License | MIT| Doc PR | ~The XSD has been missed in#48387.Commits-------742e5c2 [SecurityBundle] Add `firewalls.logout.csrf_token_manager` to XSD
Uh oh!
There was an error while loading.Please reload this page.
A long time ago,#6554 replaced
CsrfProviderInterfacebyCsrfTokenGeneratorInterface, and#9216 split the latter intoCsrfTokenManagerInterfaceandTokenGeneratorInterface.#9587 later introducedcsrf_token_generator, which was already wrong at the time.Given that token generators exist, it feels weird to have to set
csrf_token_generatortosecurity.csrf.token_manageras mentioned inthe documentation.As this confusion recently led to#48339, I propose to rename
firewalls.logout.csrf_token_generatortofirewalls.logout.csrf_token_manager.