[Security][SecurityBundle] Use csrf_token_id instead of deprecated intention #16722
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
jakzal
commented
Nov 28, 2015
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #16720 |
| License | MIT |
| Doc PR | - |
| @@ -299,7 +299,7 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a | |||
| $listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.logout_listener')); | |||
| $listener->replaceArgument(3, array( | |||
| 'csrf_parameter' => $firewall['logout']['csrf_parameter'], | |||
| 'intention' => $firewall['logout']['csrf_token_id'], | |||
| 'csrf_token_id' => $firewall['logout']['csrf_token_id'], | |||
There was a problem hiding this comment.
I think this is wrong as the LogoutListener doesn't handle an intention option (or we need to change the listener too which probably is better): https://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php#L62-L66
There was a problem hiding this comment.
I'm making changes to the security component just now :)
b194912 to
76143f0
Compare
|
@xabbuh thanks for the review! @nicolas-grekas there are some common changes made here and in the already merged #16692. Expect merge conflicts ;) |
… in favour of csrf_token_id
9985b7a to
ae4d588
Compare
|
I cleaned up the commits now. Ready to review. |
| @@ -6,6 +6,7 @@ CHANGELOG | |||
|
|
|||
| * deprecated the `key` setting of `anonymous`, `remember_me` and `http_digest` | |||
| in favor of the `secret` setting. | |||
| * deprecated the `intention` firewall listener setting in favor of the `csrf_token_id`. | |||
There was a problem hiding this comment.
Is this change also documented in the upgrade file?
|
👍 thanks! |
8798527 to
ac35350
Compare
… options are defined
|
👍 Status: Reviewed |
|
Thank you @jakzal. |
fabpot
added a commit
that referenced
this pull request
Nov 28, 2015
…f deprecated intention (jakzal) This PR was squashed before being merged into the 2.8 branch (closes #16722). Discussion ---------- [Security][SecurityBundle] Use csrf_token_id instead of deprecated intention | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #16720 | License | MIT | Doc PR | - Commits ------- 0450865 [Security][SecurityBundle] Use csrf_token_id instead of deprecated intention
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.