-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[Security] Add ability for voters to explain their vote #20690
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 7.3
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,14 +40,20 @@ or extend :class:`Symfony\\Component\\Security\\Core\\Authorization\\Voter\\Vote | |
which makes creating a voter even easier:: | ||
|
||
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; | ||
use Symfony\Component\Security\Core\Authorization\Voter\Vote; | ||
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; | ||
|
||
abstract class Voter implements VoterInterface | ||
{ | ||
abstract protected function supports(string $attribute, mixed $subject): bool; | ||
abstract protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool; | ||
abstract protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token, ?Vote $vote = null): bool; | ||
} | ||
|
||
.. versionadded:: 7.3 | ||
|
||
The `$vote` parameter in the :method:`Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::voteOnAttribute` method | ||
was introduced in Symfony 7.3. | ||
|
||
.. _how-to-use-the-voter-in-a-controller: | ||
|
||
.. tip:: | ||
|
@@ -140,6 +146,7 @@ would look like this:: | |
use App\Entity\Post; | ||
use App\Entity\User; | ||
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; | ||
use Symfony\Component\Security\Core\Authorization\Voter\Vote; | ||
use Symfony\Component\Security\Core\Authorization\Voter\Voter; | ||
|
||
class PostVoter extends Voter | ||
|
@@ -163,12 +170,14 @@ would look like this:: | |
return true; | ||
} | ||
|
||
protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool | ||
protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token, ?Vote $vote = null): bool | ||
{ | ||
$user = $token->getUser(); | ||
$vote ??= new Vote(); | ||
|
||
if (!$user instanceof User) { | ||
// the user must be logged in; if not, deny access | ||
$vote->reasons[] = 'The user is not logged in.'; | ||
MrYamous marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should instead use the $vote?->addReason('The user is not logged in.'); |
||
return false; | ||
} | ||
|
||
|
@@ -197,7 +206,13 @@ would look like this:: | |
private function canEdit(Post $post, User $user): bool | ||
{ | ||
// this assumes that the Post object has a `getOwner()` method | ||
return $user === $post->getOwner(); | ||
if ($user === $post->getOwner()) { | ||
return true; | ||
} | ||
|
||
$vote->reasons[] = 'You are not the owner of the Post.'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. $vote is undefined in this example, I guess a few more changes are needed to pass $vote |
||
|
||
return false; | ||
} | ||
} | ||
|
||
|
@@ -215,11 +230,12 @@ To recap, here's what's expected from the two abstract methods: | |
return ``true`` if the attribute is ``view`` or ``edit`` and if the object is | ||
a ``Post`` instance. | ||
|
||
``voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token)`` | ||
``voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token, ?Vote $vote = null)`` | ||
If you return ``true`` from ``supports()``, then this method is called. Your | ||
job is to return ``true`` to allow access and ``false`` to deny access. | ||
The ``$token`` can be used to find the current user object (if any). In this | ||
example, all of the complex business logic is included to determine access. | ||
The ``$token`` can be used to find the current user object (if any). The ``$vote`` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. perhaps document were this can be seen? or retrieved to be displayed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure to understand your point here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I meant something like the workflow transition blocker message There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @94noni. Having a short section explaining how to retrieve and display the reasons to users would be a great addition. It would help developers make better use of the new Vote object in real-world applications. |
||
argument can be used to add a reason to the vote. In this example, all of the | ||
complex business logic is included to determine access. | ||
|
||
.. _declaring-the-voter-as-a-service: | ||
|
||
|
@@ -256,7 +272,7 @@ with ``ROLE_SUPER_ADMIN``:: | |
) { | ||
} | ||
|
||
protected function voteOnAttribute($attribute, mixed $subject, TokenInterface $token): bool | ||
protected function voteOnAttribute($attribute, mixed $subject, TokenInterface $token, ?Vote $vote = null): bool | ||
{ | ||
// ... | ||
|
||
|
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.
should be removed while accounting for @javiereguiluz's comment: