[Security] add & update doc entries on AbstractVoter implementation #4257
Conversation
inoryy
commented
Sep 23, 2014
inoryy
commented
| Q | A |
|---|---|
| Doc fix? | no |
| New docs? | yes (related to symfony/symfony#11183) |
| Applies to | 2.6 |
| Fixed tickets | - |
| .. include:: /cookbook/security/voter_interface.rst.inc | ||
|
|
||
| The basic functionality covering common use cases is provided | ||
| and end developer is expected to implement the abstract methods. |
There was a problem hiding this comment.
end developer does not convey end user, either settle for end user or just developer
This PR was squashed before being merged into the 2.6-dev branch (closes #11183). Discussion ---------- [Security] add an AbstractVoter implementation | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT | Doc PR | symfony/symfony-docs#4257 The idea is to reduce boilerplate required to create custom Voter, doing most of the work for the developer and guiding him on the path by providing simple requirements via abstract methods that will be called by the AbstractVoter. P.S. This is meant to be a [DX Initiative](https://github.com/symfony/symfony/issues?labels=DX&state=open) improvement. Commits ------- d3bafc6 [Security] add an AbstractVoter implementation
| @@ -61,84 +62,45 @@ edit a particular object. Here's an example implementation: | |||
| // src/Acme/DemoBundle/Security/Authorization/Voter/PostVoter.php | |||
| namespace Acme\DemoBundle\Security\Authorization\Voter; | |||
|
|
|||
| use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; | |||
| use Symfony\Component\Security\Core\Authorization\Voter\AbstractVoter; | |||
| use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; | |||
There was a problem hiding this comment.
Do we need to import this TokenInterface class when using the new AbstractVoter?
|
@javiereguiluz @cordoval should be fixed now |
| @@ -92,6 +92,13 @@ the security layer. This can be done easily through the service container. | |||
| methods in your implementation of the ``vote()`` method and return ``ACCESS_ABSTAIN`` | |||
| if your voter does not support the class or attribute. | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
I think we should actually use the AbstractVoter in this example. Other than mentioning that VoterInterface exists and you can just implement it, I don't see any disadvantage to always using AbstractVoter. Agree?
There was a problem hiding this comment.
I think in this particular use case VoterInterface is actually the better choice.
AbstractVoter assumes you know exactly what attributes and classes you want to match, whereas in this situation we're meant to allow everything through (look at how supportsAttribute and supportsClass are defined).
|
@inoryy I just left some comments - sorry if it was a lot - this is just an under-appreciated feature that got a lot better! |
|
@inoryy can you please apply @weaverryan 's comments? (btw, it also needs a rebase) |
|
@inoryy Do you have time to finish this? Anyway, I wish you a happy new year. 🎉 |
Conflicts: cookbook/security/voters_data_permission.rst
|
@weaverryan @wouterj @xabbuh should be done, sorry for the long delay |
|
|
||
| The :method:`Symfony\\Component\\Security\\Core\\Authorization\\Voter\\AbstractVoter::getSupportedClasses` | ||
| method tells Symfony that your voter should be called whenever an object of one of the given classes | ||
| is passed to `isGranted` For example, if you return ['\Acme\DemoBundle\Model\Product'], |
There was a problem hiding this comment.
double-ticks around isGranted
There was a problem hiding this comment.
Change the ['Acme\DemoBundle... to be AppBundle\Model\Product and use a full array( there instead of [.
| Symfony will call your voter when a `Product` object is passed to `isGranted`. | ||
|
|
||
| The :method:`Symfony\\Component\\Security\\Core\\Authorization\\Voter\\AbstractVoter::getSupportedAttributes` | ||
| method tells Symfony that your voter should be called whenever one of these strings is passes as the |
|
Thanks Roman - one last round of changes, I promise :) |
|
Hi @inoryy. Ryan did another (last) round of comments on your PR some weeks ago. Do you have some time to update the PR, so we can merge it? If you don't, there is a doc sprint day on May 23rd were we can help you (or someone else can take the PR over and finish it). Thanks for the patience! |
|
@wouterj hello, unfortunately I'm swamped with studies right now, so yes - please do take over. Thanks! |
xabbuh
added
the
actionable
|
This PR is being finished in #5423 |
|
Thanks Javier! |
…plementation (Inoryy, javiereguiluz) This PR was submitted for the 2.7 branch but it was merged into the 2.6 branch instead (closes #5423). Discussion ---------- [Security] add & update doc entries on AbstractVoter implementation | Q | A | ------------- | --- | Doc fix? | no | New docs? | yes (symfony/symfony#11183) | Applies to | 2.6+ | Fixed tickets | - This PR finishes #4257. Commits ------- 95537d3 Added a link to the AbstractVoter cookbook 73bd908 Fixed some typos 60643f0 Removed the abstract_voter.rst.inc file 59c60b1 add fixes to abstract_voter include file e9053c0 fix problems pointed out by @javiereguiluz and @cordoval 968cb65 add & update doc entries on AbstractVoter implementation
