From 06188272de7792019246db779d788ce69c81907b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= <fromme@dreipunktnull.com>
Date: Wed, 6 Dec 2023 18:14:11 +0100
Subject: [PATCH] feat: refactor voter to improve role separation

---
 src/Security/Voter/DispositionVoter.php | 58 ++++++++++++++-----------
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/src/Security/Voter/DispositionVoter.php b/src/Security/Voter/DispositionVoter.php
index 4923838..fda3f8c 100644
--- a/src/Security/Voter/DispositionVoter.php
+++ b/src/Security/Voter/DispositionVoter.php
@@ -5,7 +5,6 @@ namespace App\Security\Voter;
 use App\BusProNet\DataProvider\HotelDataProvider;
 use App\BusProNet\Model\Hotel;
 use App\Entity\Disposition;
-use App\Entity\User;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Authorization\Voter\Voter;
 
@@ -28,27 +27,43 @@ class DispositionVoter extends Voter
             return false;
         }
 
-        return in_array($attribute, [static::VIEW, static::EDIT, static::DELETE, static::CONTRACT, static::INVOICE, static::FEEDBACK]);
+        return in_array($attribute, [
+            static::VIEW,
+            static::EDIT,
+            static::DELETE,
+            static::CONTRACT,
+            static::INVOICE,
+            static::FEEDBACK,
+        ]);
     }
 
     protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool
     {
         /** @var Disposition $disposition */
         $disposition = $subject;
-        /** @var User $user */
-        $user = $token->getUser();
 
-        // Administrative users have full access to all dispositions
-        if (in_array('ROLE_ADMINISTRATIVE', $token->getRoleNames())) {
-            return true;
+        return match ($attribute) {
+            static::VIEW, static::EDIT, static::CONTRACT, static::INVOICE => $this->assertAdministrativeAccess($token) || $this->assertTeamerAccess($token, $disposition),
+            static::DELETE => $this->assertAdministrativeAccess($token),
+            static::FEEDBACK => $this->assertHouseManagerAccess($token, $disposition),
+            default => false,
+        };
+    }
+
+    private function assertAdministrativeAccess(TokenInterface $token): bool
+    {
+        return in_array('ROLE_ADMINISTRATIVE', $token->getRoleNames());
+    }
+
+    private function assertHouseManagerAccess(TokenInterface $token, Disposition $disposition): bool
+    {
+        if (false === in_array('ROLE_HOUSE_MANAGER', $token->getRoleNames())) {
+            return false;
         }
 
-        // House managers may provide feedback for dispositions that are past
-        // and are associated with their hotel
-        if (in_array('ROLE_HOUSE_MANAGER', $token->getRoleNames()) && static::FEEDBACK === $attribute) {
             $hotels = $this
                 ->hotelDataProvider
-                ->findByCode($user->getHotelCode())
+                ->findByCode($token->getUser()->getHotelCode())
             ;
             $hotelBusProIds = array_map(function (Hotel $hotel) {
                 return $hotel->getBusProId();
@@ -60,21 +75,14 @@ class DispositionVoter extends Voter
 
             return in_array($destination->getHotelBusProId(), $hotelBusProIds)
                 && $destination->getDateTo() < new \DateTimeImmutable();
+    }
+
+    private function assertTeamerAccess(TokenInterface $token, Disposition $disposition): bool
+    {
+        if (false === in_array('ROLE_TEAMER', $token->getRoleNames())) {
+            return false;
         }
 
-        // Teamers may only view or edit their own dispositions
-        if (in_array('ROLE_TEAMER', $token->getRoleNames())) {
-            $teamer = $user->getTeamer();
-
-            switch ($attribute) {
-                case static::VIEW:
-                case static::EDIT:
-                case static::CONTRACT:
-                case static::INVOICE:
-                    return $teamer === $disposition->getTeamer();
-            }
-        }
-
-        return false;
+        return $token->getUser()->getTeamer() === $disposition->getTeamer();
     }
 }
\ No newline at end of file