Cleanup security voters cookbook recipes #5453
Merged
Conversation
This file contains 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
wouterj
force-pushed
the
issue_5084
branch
2 times, most recently
from
0c9c0f9 to
d6fb58a
Compare
| @@ -14,7 +14,7 @@ the ACL system comes in. | |||
| Using ACL's isn't trivial, and for simpler use cases, it may be overkill. | |||
| If your permission logic could be described by just writing some code (e.g. | |||
| to check if a Blog is owned by the current User), then consider using | |||
| :doc:`voters </cookbook/security/voters_data_permission>`. A voter is passed the object | |||
| :doc:`voters </cookbook/security/voters`. A voter is passed the object | |||
| use business logic (e.g. the user can edit this post because they were | ||
| the creator) to determine access. You'll probably want this option - it's | ||
| flexible enough to solve the above situation. | ||
| * :doc:`Voters </cookbook/security/voters>` allow you to use business logic |
There was a problem hiding this comment.
[...] to use business logic [...] looks very generic to me. I'd like something like: ... apply your own business logic ..., ... define the custom logic applied to ..., etc.
weaverryan
added a commit
that referenced
this pull request
Jul 16, 2015
This PR was merged into the 2.3 branch. Discussion ---------- Cleanup security voters cookbook recipes | Q | A | --- | --- | Doc fix? | yes | New docs? | no | Applies to | all | Fixed tickets | #5084 Blacklisting using voters isn't a good practice and in fact, the article (`cookbook/security/voters`) was almost completely equal to the much newer voter article (`cookbook/security/voters_data_permission`). I've moved the only different piece (access decision strategy) to the newer voter article and then removed the old voter article. As the old voter article had a much nicer URL than the new one, I then renamed the new article to have the better name and set up a redirection. A [quick search on Google](https://www.google.nl/search?q=%5Blink%3Ahttp%3A%2F%2Fsymfony.com%2Fdoc%2Fcurrent%2Fcookbook%2Fsecurity%2Fvoters.html%5D+-site%3Ahttp%3A%2F%2Fsymfony.com) revealed that there are almost no links to the old article talking about blacklisting specifically, just about voters. So I think we're good here. The diff is horrible, that's why I've tried to create multiple commits (this will ease reviewing process). The only commit requiring review is 9c169c7 (as the other 2 are just moving and removing stuff). Commits ------- 93484a7 Remove the old voter article 9c169c7 Rewrite new section acf66f9 Move access decision strategy section
weaverryan
added a commit
that referenced
this pull request
Jul 16, 2015
weaverryan
added a commit
that referenced
this pull request
Jul 16, 2015
* 2.3: [#5453] Minor tweaks - mostly thanks to Javier
weaverryan
added a commit
that referenced
this pull request
Jul 16, 2015
* 2.6: [#5472] Minor tweak and adding code example [#5453] Minor tweaks - mostly thanks to Javier Reword Fix caching Fix build Added a tip about hashing the result of nextBytes() rework the quick tour's big picture fix for Symfony 2.7 Fix after install URL and new photo since AcmeDemoBundle is not part of 2.7 Improve travis build speed
weaverryan
added a commit
that referenced
this pull request
Jul 16, 2015
* 2.7: [#5472] Minor tweak and adding code example [#5453] Minor tweaks - mostly thanks to Javier Reword Fix caching Fix build Added a tip about hashing the result of nextBytes() rework the quick tour's big picture fix for Symfony 2.7 Fix after install URL and new photo since AcmeDemoBundle is not part of 2.7 Improve travis build speed
weaverryan
added a commit
that referenced
this pull request
Jul 16, 2015
* 2.8: [#5472] Minor tweak and adding code example [#5453] Minor tweaks - mostly thanks to Javier Reword Fix moving the mapped down as per @xabbuh Fix caching Fix build Fix the rendering issue Fix issues reported by @snoek09 . Thank you. Added a tip about hashing the result of nextBytes() Make the necessary changes mentioned by @xabbuh and add example for form rework the quick tour's big picture fix for Symfony 2.7 Fix after install URL and new photo since AcmeDemoBundle is not part of 2.7 Starting with range documentation Improve travis build speed
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.
Blacklisting using voters isn't a good practice and in fact, the article (
cookbook/security/voters) was almost completely equal to the much newer voter article (cookbook/security/voters_data_permission). I've moved the only different piece (access decision strategy) to the newer voter article and then removed the old voter article.As the old voter article had a much nicer URL than the new one, I then renamed the new article to have the better name and set up a redirection. A quick search on Google revealed that there are almost no links to the old article talking about blacklisting specifically, just about voters. So I think we're good here.
The diff is horrible, that's why I've tried to create multiple commits (this will ease reviewing process). The only commit requiring review is 9c169c7 (as the other 2 are just moving and removing stuff).