Skip to content

[Security][TwigBridge] Add access_decision() and access_decision_for_user() #61379

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

Open
wants to merge 3 commits into
base: 7.4
Choose a base branch
from

Conversation

florentdestremau
Copy link
Contributor

@florentdestremau florentdestremau commented Aug 10, 2025

Q A
Branch? 7.4
Bug fix? no
New feature? yes
Deprecations? no
Issues
License MIT

This PR adds 2 new methods and the corresponding twig helpers to retrieve the access decision from a Voter rather than simply the result. This way, you can enumerate the AccessDecision::votes and retrieve the votes and their reason.

For instance, in a controller or service:

        $accessDecision = $this->security->accessDecision('post_edit', $post);
        /** @var array<Vote> $votes */
        $votes = array_filter(
            $accessDecision->votes,
            fn (Vote $vote) => VoterInterface::ACCESS_DENIED === $vote->result,
        );
        if (!$accessDecision->isGranted) {
            $message = "You don't have access to this post\n";
            foreach ($votes as $vote) {
                $message .= join(', ', $vote->reasons) . '\n';
            }
            $this->addFlash('danger', $message);
        }

Same for a user on his behalf, and same in Twig;

{% set access = access_decision('post_edit', post) %}

<a href="/post/123/edit" 
    {% if not access.isGranted %}
    title="You don't have access to this post : {{ access.votes|filter(v => v.result = -1)|map(v => v.reasons|join(',')) }}"
    disabled
    {% endif %}
>
  Edit this post
</a>    

Given that the main motivation for this is to explain in an API or in twig the reasons why the access is not granted, I'm tempted to pre-filter the failed votes already, or add an extra getter AccessDecision::failedVotes to help create these small extensions ?

Side note: 2 extra commits given that I started in 7.3, will fix if this picks up interest

antalaron and others added 3 commits August 5, 2025 13:38
…ld (antalaron)

This PR was merged into the 7.3 branch.

Discussion
----------

[Mailer] [Resend] Add friendly name in the `to` field

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | n/a
| License       | MIT

This PR adds support for friendly email address formatting (e.g. "John Doe <john@example.com>") in the `to` field.

Although Resend's API accepts this format natively, the current implementation of the bridge does not pass it through correctly — names are silently discarded and only raw email addresses are used. This fix ensures that display names are preserved and passed to the API as intended.

Commits
-------

f9c0a3d Add friendly name in the `to` field
This new function returns an AccessDecision object, allowing for further
treatment in user land such as collecting the vote reasons.
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks useful to me
some tests + changelog entries would be needed

if ($attachements = $this->prepareAttachments($email)) {
$payload['attachments'] = $attachements;
if ($attachments = $this->prepareAttachments($email)) {
$payload['attachments'] = $attachments;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like a rebase is needed :)

Comment on lines +58 to +60
public function accessDecision(mixed $role, mixed $object = null, ?string $field = null, ?AccessDecision $accessDecision = null): AccessDecision
{
$accessDecision = $accessDecision ?? new AccessDecision();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not there's a point for templates to pass a previous access decision:

Suggested change
public function accessDecision(mixed $role, mixed $object = null, ?string $field = null, ?AccessDecision $accessDecision = null): AccessDecision
{
$accessDecision = $accessDecision ?? new AccessDecision();
public function accessDecision(mixed $role, mixed $object = null, ?string $field = null): AccessDecision
{
$accessDecision = new AccessDecision();

Comment on lines +93 to +95
public function accessDecisionForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?string $field = null, ?AccessDecision $accessDecision = null): AccessDecision
{
$accessDecision = $accessDecision ?? new AccessDecision();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function accessDecisionForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?string $field = null, ?AccessDecision $accessDecision = null): AccessDecision
{
$accessDecision = $accessDecision ?? new AccessDecision();
public function accessDecisionForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?string $field = null): AccessDecision
{
$accessDecision = new AccessDecision();

Comment on lines +68 to +70
public function accessDecision(mixed $attributes, mixed $subject = null, ?AccessDecision $accessDecision = null): AccessDecision
{
$accessDecision = $accessDecision ?? new AccessDecision();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function accessDecision(mixed $attributes, mixed $subject = null, ?AccessDecision $accessDecision = null): AccessDecision
{
$accessDecision = $accessDecision ?? new AccessDecision();
public function accessDecision(mixed $attributes, mixed $subject = null): AccessDecision
{
$accessDecision = new AccessDecision();

Comment on lines +87 to +89
public function accessDecisionForUser(UserInterface $user, mixed $attributes, mixed $subject = null, ?AccessDecision $accessDecision = null): AccessDecision
{
$accessDecision = $accessDecision ?? new AccessDecision();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function accessDecisionForUser(UserInterface $user, mixed $attributes, mixed $subject = null, ?AccessDecision $accessDecision = null): AccessDecision
{
$accessDecision = $accessDecision ?? new AccessDecision();
public function accessDecisionForUser(UserInterface $user, mixed $attributes, mixed $subject = null): AccessDecision
{
$accessDecision = new AccessDecision();

@nicolas-grekas nicolas-grekas changed the title [Security] New methods: access decision and access decision for user [Security][TwigBundle] access decision and access decision for user Aug 19, 2025
@nicolas-grekas nicolas-grekas changed the title [Security][TwigBundle] access decision and access decision for user [Security][TwigBridge] Add access_decision() and access_decision_for_user() Aug 19, 2025
@nicolas-grekas
Copy link
Member

Given that the main motivation for this is to explain in an API or in twig the reasons why the access is not granted, I'm tempted to pre-filter the failed votes already, or add an extra getter AccessDecision::failedVotes to help create these small extensions ?

this is already covered by the AccessDecision::getMessage method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants