[Security] Vote can have multiple messages

symfony · alamirault · May 26, 2022 · Jun 28, 2022 · Mar 10, 2023 · Sep 5, 2023
commit 5e4733be766c03c5f2cf71d6d298c2d6a9d80cc5
@@ -11,26 +11,36 @@
 
 namespace Symfony\Component\Security\Core\Authorization\Voter;
 
+use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
+
 /**
  * A Vote is returned by a Voter and contains the access (granted, abstain or denied).
- * It can also contain a message explaining the vote decision.
+ * It can also contain one or multiple messages explaining the vote decision.
  *
  * @author Dany Maillard <danymaillard93b@gmail.com>
+ * @author Antoine Lamirault <lamiraultantoine@gmail.com>
  */
 final class Vote
 {
-    /** @var int One of the VoterInterface::ACCESS_* constants */
-    private $access;
-    private $message;
-    private $context;
+    /**
+     * @var int One of the VoterInterface::ACCESS_* constants
+     */
+    private int $access;
+
+    /**
+     * @var string[]
+     */
+    private array $messages;
+
+    private array $context;
 
     /**
      * @param int $access One of the VoterInterface::ACCESS_* constants
      */
-    public function __construct(int $access, string $message = '', array $context = [])
+    public function __construct(int $access, string|array $messages = [], array $context = [])
     {
         $this->access = $access;
-        $this->message = $message;
+        $this->setMessages($messages);
         $this->context = $context;
     }
 
@@ -54,34 +64,67 @@ public function isDenied(): bool
         return VoterInterface::ACCESS_DENIED === $this->access;
     }
 
-    public static function create(int $access, string $message = '', array $context = []): self
+    /**
+     * @param string|string[] $messages
+     */
+    public static function create(int $access, string|array $messages = [], array $context = []): self
     {
-        return new self($access, $message, $context);
+        return new self($access, $messages, $context);
     }
 
-    public static function createGranted(string $message = '', array $context = []): self
+    /**
+     * @param string|string[] $messages
+     */
+    public static function createGranted(string|array $messages = [], array $context = []): self
     {
-        return new self(VoterInterface::ACCESS_GRANTED, $message, $context);
+        return new self(VoterInterface::ACCESS_GRANTED, $messages, $context);
     }
 
-    public static function createAbstain(string $message = '', array $context = []): self
+    /**
+     * @param string|string[] $messages
+     */
+    public static function createAbstain(string|array $messages = [], array $context = []): self
     {
-        return new self(VoterInterface::ACCESS_ABSTAIN, $message, $context);
+        return new self(VoterInterface::ACCESS_ABSTAIN, $messages, $context);
     }
 
-    public static function createDenied(string $message = '', array $context = []): self
+    /**
+     * @param string|string[] $messages
+     */
+    public static function createDenied(string|array $messages = [], array $context = []): self
+    {
+        return new self(VoterInterface::ACCESS_DENIED, $messages, $context);
+    }
+
+    /**
+     * @param string|string[] $messages
+     */
+    public function setMessages(string|array $messages)
+    {
+        $this->messages = (array) $messages;
+        foreach ($this->messages as $message) {
+            if (!\is_string($message)) {
+                throw new InvalidArgumentException(sprintf('Message must be string, "%s" given.', get_debug_type($message)));
+            }
+        }
+    }
+
+    public function addMessage(string $message)
     {
-        return new self(VoterInterface::ACCESS_DENIED, $message, $context);
+        $this->messages[] = $message;
     }
 
-    public function setMessage(string $message)
+    /**
+     * @return string[]
+     */
+    public function getMessages(): array
     {
-        $this->message = $message;
+        return $this->messages;
     }
 
     public function getMessage(): string
     {
-        return $this->message;
+        return implode(', ', $this->messages);
     }
 
     public function getContext(): array

@@ -43,7 +43,9 @@ public function getVote(TokenInterface $token, mixed $subject, array $attributes
             }
 
             // as soon as at least one attribute is supported, default is to deny access
-            $vote = $this->deny();
+            if (!$vote->isDenied()) {
+                $vote = $this->deny();
+            }
 
             $decision = $this->voteOnAttribute($attribute, $subject, $token);
             if (\is_bool($decision)) {
@@ -56,7 +58,9 @@ public function getVote(TokenInterface $token, mixed $subject, array $attributes
                 return $decision;
             }
 
-            $vote->setMessage($vote->getMessage().trim(' '.$decision->getMessage()));
+            if ('' !== $decision->getMessage()) {
+                $vote->addMessage($decision->getMessage());
+            }
         }
 
         return $vote;
@@ -71,26 +75,32 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes):
 
     /**
      * Creates a granted vote.
+     *
+     * @param string|string[] $messages
      */
-    protected function grant(string $message = '', array $context = []): Vote
+    protected function grant(string|array $messages = [], array $context = []): Vote
     {
-        return Vote::createGranted($message, $context);
+        return Vote::createGranted($messages, $context);
     }
 
     /**
      * Creates an abstained vote.
+     *
+     * @param string|string[] $messages
      */
-    protected function abstain(string $message = '', array $context = []): Vote
+    protected function abstain(string|array $messages = [], array $context = []): Vote
     {
-        return Vote::createAbstain($message, $context);
+        return Vote::createAbstain($messages, $context);
     }
 
     /**
      * Creates a denied vote.
+     *
+     * @param string|string[] $messages
      */
-    protected function deny(string $message = '', array $context = []): Vote
+    protected function deny(string|array $messages = [], array $context = []): Vote
     {
-        return Vote::createDenied($message, $context);
+        return Vote::createDenied($messages, $context);
     }
 
     /**

@@ -0,0 +1,38 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Core\Tests\Authorization\Voter;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Security\Core\Authorization\Voter\Vote;
+use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
+use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
+
+class VoteTest extends TestCase
+{
+    public function testMessages()
+    {
+        $vote = new Vote(VoterInterface::ACCESS_GRANTED, 'foo');
+
+        $this->assertSame('foo', $vote->getMessage());
+
+        $vote->addMessage('bar');
+        $this->assertSame('foo, bar', $vote->getMessage());
+    }
+
+    public function testMessagesWithNotString()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('Message must be string, "bool" given.');
+
+        new Vote(VoterInterface::ACCESS_GRANTED, ['foo', true]);
+    }
+}
@@ -110,10 +110,17 @@ public function testVoteLegacy(VoterInterface $voter, array $attributes, $expect
     public function testVoteMessage()
     {
         $voter = new IntegerVoterVoteTest_Voter();
-        $vote = $voter->getVote($this->token, new \stdClass(), [43, 44]);
+        $vote = $voter->getVote($this->token, new \stdClass(), [43]);
         $this->assertSame('foobar message', $vote->getMessage());
     }
 
+    public function testVoteMessageMultipleAttributes()
+    {
+        $voter = new IntegerVoterVoteTest_Voter();
+        $vote = $voter->getVote($this->token, new \stdClass(), [43, 44]);
+        $this->assertSame('foobar message, foobar message', $vote->getMessage());
+    }
+
     public function testGetVoteWithTypeError()
     {
         $this->expectException(\TypeError::class);
@@ -183,7 +190,7 @@ protected function voteOnAttribute($attribute, $object, TokenInterface $token):
             return Vote::createGranted();
         }
 
-        return Vote::createDenied(' foobar message ');
+        return Vote::createDenied('foobar message');
     }
 
     protected function supports($attribute, $object): bool