From 5fed42a623fdd163a8733e481f779ad2a526292a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julien=20Fastr=C3=A9?= <julien.fastre@champs-libres.coop>
Date: Thu, 25 Jul 2024 23:34:36 +0200
Subject: [PATCH] Fix decision logic in AbstractStoredObjectVoter

Amend the condition to ensure proper attribute validation before checking workflow association. This prevents unintended execution paths and potential exceptions when the workflow document service is not provided.
---
 .../Security/Authorization/StoredObjectVoter.php    | 13 +++++++++++--
 .../StoredObjectVoter/AbstractStoredObjectVoter.php |  2 +-
 .../Authorization/StoredObjectVoterTest.php         |  3 ++-
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php
index 91e767af2..17ba1f59d 100644
--- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php
+++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php
@@ -12,6 +12,7 @@ declare(strict_types=1);
 namespace Chill\DocStoreBundle\Security\Authorization;
 
 use Chill\DocStoreBundle\Entity\StoredObject;
+use Psr\Log\LoggerInterface;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Authorization\Voter\Voter;
 use Symfony\Component\Security\Core\Security;
@@ -23,7 +24,9 @@ use Symfony\Component\Security\Core\Security;
  */
 class StoredObjectVoter extends Voter
 {
-    public function __construct(private readonly Security $security, private readonly iterable $storedObjectVoters) {}
+    public const LOG_PREFIX = '[stored object voter] ';
+
+    public function __construct(private readonly Security $security, private readonly iterable $storedObjectVoters, private readonly LoggerInterface $logger) {}
 
     protected function supports($attribute, $subject): bool
     {
@@ -39,7 +42,13 @@ class StoredObjectVoter extends Voter
         // Loop through context-specific voters
         foreach ($this->storedObjectVoters as $storedObjectVoter) {
             if ($storedObjectVoter->supports($attributeAsEnum, $subject)) {
-                return $storedObjectVoter->voteOnAttribute($attributeAsEnum, $subject, $token);
+                $grant = $storedObjectVoter->voteOnAttribute($attributeAsEnum, $subject, $token);
+
+                if (false === $grant) {
+                    $this->logger->debug(self::LOG_PREFIX.'deny access by storedObjectVoter', ['stored_object_voter' => get_class($storedObjectVoter)]);
+                }
+
+                return $grant;
             }
         }
 
diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter/AbstractStoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter/AbstractStoredObjectVoter.php
index 2e76da318..9d77211c2 100644
--- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter/AbstractStoredObjectVoter.php
+++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter/AbstractStoredObjectVoter.php
@@ -56,7 +56,7 @@ abstract class AbstractStoredObjectVoter implements StoredObjectVoterInterface
             return false;
         }
 
-        if ($this->canBeAssociatedWithWorkflow()) {
+        if (StoredObjectRoleEnum::SEE !== $attribute && $this->canBeAssociatedWithWorkflow()) {
             if (null === $this->workflowDocumentService) {
                 throw new \LogicException('Provide a workflow document service');
             }
diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Security/Authorization/StoredObjectVoterTest.php b/src/Bundle/ChillDocStoreBundle/Tests/Security/Authorization/StoredObjectVoterTest.php
index a113cfb7b..b9a025f45 100644
--- a/src/Bundle/ChillDocStoreBundle/Tests/Security/Authorization/StoredObjectVoterTest.php
+++ b/src/Bundle/ChillDocStoreBundle/Tests/Security/Authorization/StoredObjectVoterTest.php
@@ -17,6 +17,7 @@ use Chill\DocStoreBundle\Security\Authorization\StoredObjectVoter;
 use Chill\DocStoreBundle\Security\Authorization\StoredObjectVoterInterface;
 use Chill\MainBundle\Entity\User;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
 use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
@@ -43,7 +44,7 @@ class StoredObjectVoterTest extends TestCase
             ->with($this->logicalOr($this->identicalTo('ROLE_USER'), $this->identicalTo('ROLE_ADMIN')))
             ->willReturn($securityIsGrantedResult);
 
-        $voter = new StoredObjectVoter($security, $storedObjectVoters);
+        $voter = new StoredObjectVoter($security, $storedObjectVoters, new NullLogger());
 
         self::assertEquals($expected, $voter->vote($token, $subject, [$attribute]));
     }