Add a security Post Voter #442
Conversation
| */ | ||
| class PostVoter extends Voter | ||
| { | ||
| const VIEW = 'show'; |
There was a problem hiding this comment.
I have doubts about this permission, since in the real world any user should see any Post. This is restricted in admin only but is a bit confusing.
| if (null === $this->getUser() || !$post->isAuthor($this->getUser())) { | ||
| throw $this->createAccessDeniedException('Posts can only be edited by their authors.'); | ||
| } | ||
| $this->denyAccessUnlessGranted('edit', $post, 'Posts can only be edited by their authors.'); |
There was a problem hiding this comment.
We should use PostVoter::EDIT instead?
| /** | ||
| * Is the given User the author of this Post? | ||
| */ | ||
| public function isAuthor(User $user) |
There was a problem hiding this comment.
We should keep it to implement both ways?
|
Rebased with master! |
| protected function supports($attribute, $subject) | ||
| { | ||
| // this voter is only executed for three specific permissions on Post objects | ||
| return $subject instanceof Post && in_array($attribute, [self::SHOW, self::EDIT, self::DELETE]); |
There was a problem hiding this comment.
I've simplified this method to a 1-liner to reduce the perceived complexity of voters. They have been traditionally criticized for the required boilerplate ... and thanks to the new abstract voter, it's code can be super simple.
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| protected function voteOnAttribute($attribute, $post, TokenInterface $token) |
There was a problem hiding this comment.
This implementation can be simplified too, just one line:
return $token->getUser() === $post->getAuthor();
? because $post->getAuthor() always has an author when $token->getUser() is null this return false.
There was a problem hiding this comment.
What you say is correct ... but at the same time is very common to do the if (!$user instanceof User) { return false; } thing in voters ... so we probably should keep it.
|
@yceruto I wanted to add a security voter since day one. Thanks for making it happen! |
This PR was merged into the master branch. Discussion ---------- Add a security Post Voter Implement #440 feature. - [x] Add more code comments and doc references Following some references and recommendations in: - https://symfony.com/doc/current/components/security/authorization.html - http://symfony.com/doc/current/security/voters.html - https://stovepipe.systems/post/symfony-security-roles-vs-voters (@iltar blog) Any suggestion is welcome! Commits ------- a592eea Minor reword in the help note 8468423 Simplified the voter code a bit c14e9db Add security post voter
Implement #440 feature.
Following some references and recommendations in:
Any suggestion is welcome!