refactor ACL for easy voter. Apply on TaskVoter

src/Bundle/ChillDocStoreBundle/Security/Authorization/PersonDocumentVoter.php

@@ -23,6 +23,7 @@ use Chill\MainBundle\Security\Authorization\AbstractChillVoter;
 use Chill\MainBundle\Security\Authorization\AuthorizationHelper;
 use Chill\MainBundle\Security\ProvideRoleHierarchyInterface;
 use Chill\DocStoreBundle\Entity\PersonDocument;
+use Chill\MainBundle\Security\Resolver\CenterResolverDispatcher;
 use Chill\PersonBundle\Entity\Person;
 use Chill\MainBundle\Entity\User;
 use Chill\PersonBundle\Security\Authorization\PersonVoter;
@@ -42,30 +43,25 @@ class PersonDocumentVoter extends AbstractChillVoter implements ProvideRoleHiera
     const UPDATE = 'CHILL_PERSON_DOCUMENT_UPDATE';
     const DELETE = 'CHILL_PERSON_DOCUMENT_DELETE';
 
-    /**
+    protected AuthorizationHelper $authorizationHelper;
-     * @var AuthorizationHelper
-     */
-    protected $authorizationHelper;
 
-    /**
+    protected AccessDecisionManagerInterface $accessDecisionManager;
-     * @var AccessDecisionManagerInterface
-     */
-    protected $accessDecisionManager;
 
-    /**
+    protected LoggerInterface $logger;
-     * @var LoggerInterface
+
-     */
+    protected CenterResolverDispatcher $centerResolverDispatcher;
-    protected $logger;
 
     public function __construct(
         AccessDecisionManagerInterface $accessDecisionManager,
         AuthorizationHelper $authorizationHelper,
-        LoggerInterface $logger
+        LoggerInterface $logger//,
+        //CenterResolverDispatcher $centerResolverDispatcher
     )
     {
         $this->accessDecisionManager = $accessDecisionManager;
         $this->authorizationHelper = $authorizationHelper;
         $this->logger = $logger;
+        //$this->centerResolverDispatcher = $centerResolverDispatcher;
     }
 
     public function getRoles()
@@ -85,7 +81,8 @@ class PersonDocumentVoter extends AbstractChillVoter implements ProvideRoleHiera
             return true;
         }
 
-        if ($subject instanceof Person && $attribute === self::CREATE) {
+        if ($subject instanceof Person
+            && \in_array($attribute, [self::CREATE, self::SEE])) {
             return true;
         }
 
@@ -107,6 +104,8 @@ class PersonDocumentVoter extends AbstractChillVoter implements ProvideRoleHiera
             return false;
         }
 
+        $center = $this->centerResolverDispatcher->resolveCenter($subject);
+
         if ($subject instanceof PersonDocument) {
             return $this->authorizationHelper->userHasAccess($token->getUser(), $subject, $attribute);
 

src/Bundle/ChillMainBundle/Form/Event/CustomizeFormEvent.php

@@ -0,0 +1,36 @@
+<?php
+
+namespace Chill\MainBundle\Form\Event;
+
+use Symfony\Component\Form\FormBuilderInterface;
+
+class CustomizeFormEvent extends \Symfony\Component\EventDispatcher\Event
+{
+    const NAME = 'chill_main.customize_form';
+
+    protected string $type;
+
+    protected FormBuilderInterface $builder;
+
+    public function __construct(string $type, FormBuilderInterface $builder)
+    {
+        $this->type = $type;
+        $this->builder = $builder;
+    }
+
+    /**
+     * @return string
+     */
+    public function getType(): string
+    {
+        return $this->type;
+    }
+
+    /**
+     * @return FormBuilderInterface
+     */
+    public function getBuilder(): FormBuilderInterface
+    {
+        return $this->builder;
+    }
+}

src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php

@@ -23,6 +23,8 @@ use Chill\MainBundle\Entity\User;
 use Chill\MainBundle\Entity\Center;
 use Chill\MainBundle\Entity\HasCenterInterface;
 use Chill\MainBundle\Entity\HasScopeInterface;
+use Chill\MainBundle\Security\Resolver\CenterResolverDispatcher;
+use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface;
 use Symfony\Component\Security\Core\Role\RoleHierarchyInterface;
 use Symfony\Component\Security\Core\Role\Role;
 use Chill\MainBundle\Entity\Scope;
@@ -40,11 +42,7 @@ use Chill\MainBundle\Entity\RoleScope;
  */
 class AuthorizationHelper
 {
-    /**
+    protected RoleHierarchyInterface $roleHierarchy;
-     *
-     * @var RoleHierarchyInterface
-     */
-    protected $roleHierarchy;
 
     /**
      * The role in a hierarchy, given by the parameter
@@ -52,41 +50,54 @@ class AuthorizationHelper
      *
      * @var string[]
      */
-    protected $hierarchy;
+    protected array $hierarchy;
 
-    /**
+    protected EntityManagerInterface $em;
-     *
+
-     * @var EntityManagerInterface
+    protected CenterResolverDispatcher $centerResolverDispatcher;
-     */
-    protected $em;
 
     public function __construct(
         RoleHierarchyInterface $roleHierarchy,
-        $hierarchy,
+        ParameterBagInterface $parameterBag,
-        EntityManagerInterface $em
+        EntityManagerInterface $em,
+        CenterResolverDispatcher $centerResolverDispatcher
     ) {
         $this->roleHierarchy = $roleHierarchy;
-        $this->hierarchy     = $hierarchy;
+        $this->hierarchy     = $parameterBag->get('security.role_hierarchy.roles');
         $this->em = $em;
+        $this->centerResolverDispatcher = $centerResolverDispatcher;
     }
 
     /**
      * Determines if a user is active on this center
      *
+     * If
+     *
      * @param User $user
-     * @param Center $center
+     * @param Center|Center[] $center May be an array of center
      * @return bool
      */
-    public function userCanReachCenter(User $user, Center $center)
+    public function userCanReachCenter(User $user, $center)
     {
-        foreach ($user->getGroupCenters() as $groupCenter) {
+        if ($center instanceof \Traversable) {
-            if ($center->getId() === $groupCenter->getCenter()->getId()) {
+            foreach ($center as $c) {
-                
+                if ($c->userCanReachCenter($user, $c)) {
-                return true;
+                    return true;
+                }
             }
+            return false;
+        } elseif ($center instanceof Center) {
+            foreach ($user->getGroupCenters() as $groupCenter) {
+                if ($center->getId() === $groupCenter->getCenter()->getId()) {
+                    return true;
+                }
+            }
+
+            return false;
         }
 
-        return false;
+        throw new \UnexpectedValueException(sprintf("The entity given is not an ".
+            "instance of %s, %s given", Center::class, get_class($center)));
     }
 
     /**
@@ -97,14 +108,15 @@ class AuthorizationHelper
      * the scope is taken into account.
      *
      * @param User $user
-     * @param HasCenterInterface $entity the entity may also implement HasScopeInterface
+     * @param mixed $entity the entity may also implement HasScopeInterface
      * @param string|Role $attribute
      * @return boolean true if the user has access
      */
-    public function userHasAccess(User $user, HasCenterInterface $entity, $attribute)
+    public function userHasAccess(User $user, $entity, $attribute)
     {
-        
+        if (NULL === $center = $this->centerResolverDispatcher->resolveCenter($entity)) {
-        $center = $entity->getCenter();
+            return false;
+        }
 
         if (!$this->userCanReachCenter($user, $center)) {
             return false;
@@ -112,7 +124,7 @@ class AuthorizationHelper
 
         foreach ($user->getGroupCenters() as $groupCenter){
             //filter on center
-            if ($groupCenter->getCenter()->getId() === $entity->getCenter()->getId()) {
+            if ($groupCenter->getCenter() === $center) {
                 $permissionGroup = $groupCenter->getPermissionsGroup();
                 //iterate on roleScopes
                 foreach($permissionGroup->getRoleScopes() as $roleScope) {

src/Bundle/ChillMainBundle/Security/Authorization/DefaultVoter.php

@@ -0,0 +1,69 @@
+<?php
+
+namespace Chill\MainBundle\Security\Authorization;
+
+use Chill\MainBundle\Entity\User;
+use Chill\MainBundle\Security\Resolver\CenterResolverDispatcher;
+
+class DefaultVoter implements VoterInterface
+{
+    protected AuthorizationHelper $authorizationHelper;
+
+    protected CenterResolverDispatcher $centerResolverDispatcher;
+
+    protected array $configuration = [];
+
+    /**
+     * @param AuthorizationHelper $authorizationHelper
+     * @param CenterResolverDispatcher $centerResolverDispatcher
+     * @param array $configuration
+     */
+    public function __construct(
+        AuthorizationHelper $authorizationHelper,
+        CenterResolverDispatcher $centerResolverDispatcher,
+        array $configuration
+    ) {
+        $this->authorizationHelper = $authorizationHelper;
+        $this->centerResolverDispatcher = $centerResolverDispatcher;
+        $this->configuration = $configuration;
+    }
+
+    public function supports($attribute, $subject): bool
+    {
+        foreach ($this->configuration as list($attributes, $subj)) {
+            if ($subj === null) {
+                if ($subject === null && \in_array($attribute, $attributes)) {
+                    return true;
+                }
+            } elseif ($subject instanceof $subj) {
+                return \in_array($attribute, $attributes);
+            }
+        }
+
+        return false;
+    }
+
+    public function voteOnAttribute($attribute, $subject, $token): bool
+    {
+        if (!$token->getUser() instanceof User) {
+            return false;
+        }
+
+        if (NULL === $subject) {
+           if (NULL === $center = $this->centerResolverDispatcher
+               ->resolveCenter($subject)) {
+               return false;
+           }
+           return $this->authorizationHelper->userCanReachCenter(
+               $token->getUser(),
+               $center
+           );
+        }
+
+        return $this->authorizationHelper->userHasAccess(
+            $token->getUser(),
+            $subject,
+            $attribute
+        );
+    }
+}

src/Bundle/ChillMainBundle/Security/Authorization/DefaultVoterFactory.php

@@ -0,0 +1,27 @@
+<?php
+
+namespace Chill\MainBundle\Security\Authorization;
+
+use Chill\MainBundle\Security\Resolver\CenterResolverDispatcher;
+
+class DefaultVoterFactory implements VoterFactoryInterface
+{
+    protected AuthorizationHelper $authorizationHelper;
+    protected CenterResolverDispatcher $centerResolverDispatcher;
+
+    public function __construct(
+        AuthorizationHelper $authorizationHelper,
+        CenterResolverDispatcher $centerResolverDispatcher
+    ) {
+        $this->authorizationHelper = $authorizationHelper;
+        $this->centerResolverDispatcher = $centerResolverDispatcher;
+    }
+
+    public function generate($context): VoterGeneratorInterface
+    {
+        return new VoterGenerator(
+            $this->authorizationHelper,
+            $this->centerResolverDispatcher
+        );
+    }
+}

src/Bundle/ChillMainBundle/Security/Authorization/VoterFactoryInterface.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Chill\MainBundle\Security\Authorization;
+
+interface VoterFactoryInterface
+{
+}

src/Bundle/ChillMainBundle/Security/Authorization/VoterGenerator.php

@@ -0,0 +1,36 @@
+<?php
+
+namespace Chill\MainBundle\Security\Authorization;
+
+use Chill\MainBundle\Security\Resolver\CenterResolverDispatcher;
+
+final class VoterGenerator implements VoterGeneratorInterface
+{
+    protected AuthorizationHelper $authorizationHelper;
+    protected CenterResolverDispatcher $centerResolverDispatcher;
+    protected array $configuration = [];
+
+    public function __construct(
+        AuthorizationHelper $authorizationHelper,
+        CenterResolverDispatcher $centerResolverDispatcher
+    ) {
+        $this->authorizationHelper = $authorizationHelper;
+        $this->centerResolverDispatcher = $centerResolverDispatcher;
+    }
+
+    public function addCheckFor($subject, $attributes): self
+    {
+       $this->configuration[] = [$attributes, $subject];
+
+       return $this;
+    }
+
+    public function build(): VoterInterface
+    {
+        return new DefaultVoter(
+            $this->authorizationHelper,
+            $this->centerResolverDispatcher,
+            $this->configuration
+        );
+    }
+}

src/Bundle/ChillMainBundle/Security/Authorization/VoterGeneratorInterface.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Chill\MainBundle\Security\Authorization;
+
+interface VoterGeneratorInterface
+{
+}

src/Bundle/ChillMainBundle/Security/Authorization/VoterInterface.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Chill\MainBundle\Security\Authorization;
+
+interface VoterInterface
+{
+}

src/Bundle/ChillMainBundle/Tests/Security/Resolver/CenterResolverDispatcherTest.php

@@ -0,0 +1,27 @@
+<?php
+
+namespace Chill\MainBundle\Tests\Security\Resolver;
+
+use Chill\MainBundle\Entity\Center;
+use Chill\MainBundle\Security\Resolver\CenterResolverDispatcher;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+
+class CenterResolverDispatcherTest extends KernelTestCase
+{
+    private CenterResolverDispatcher $dispatcher;
+
+    protected function setUp()
+    {
+        self::bootKernel();
+        $this->dispatcher = self::$container->get(CenterResolverDispatcher::class);
+    }
+
+    public function testResolveCenter()
+    {
+       $center = new Center();
+
+       $resolved = $this->dispatcher->resolveCenter($center);
+
+       $this->assertSame($center, $resolved);
+    }
+}

src/Bundle/ChillMainBundle/config/services/security.yaml

@@ -3,12 +3,38 @@ services:
         autowire: true
         autoconfigure: true
 
+    # do not autowire the directory Security/Resolver
+    Chill\MainBundle\Security\Resolver\CenterResolverDispatcher:
+        arguments:
+            - !tagged_iterator chill_main.center_resolver
+
+    # do not autowire the directory Security/Resolver
+    _instanceof:
+        Chill\MainBundle\Security\Resolver\CenterResolverInterface:
+            tags:
+                - chill_main.center_resolver
+
+    # do not autowire the directory Security/Resolver
+    Chill\MainBundle\Security\Resolver\DefaultCenterResolver:
+        autoconfigure: true
+        autowire: true
+
+    # do not autowire the directory Security/Resolver
+    Chill\MainBundle\Security\Resolver\ResolverTwigExtension:
+        autoconfigure: true
+        autowire: true
+
+    # do not autowire the directory Security/Resolver
+    Chill\MainBundle\Security\Authorization\DefaultVoterFactory:
+        autowire: true
+
+    # do not autowire the directory Security/Resolver
+    Chill\MainBundle\Security\Authorization\VoterFactoryInterface: '@Chill\MainBundle\Security\Authorization\DefaultVoterFactory'
+
     chill.main.security.authorization.helper:
         class: Chill\MainBundle\Security\Authorization\AuthorizationHelper
-        arguments:
+        autowire: true
-            $roleHierarchy: "@security.role_hierarchy"
+        autoconfigure: true
-            $hierarchy: "%security.role_hierarchy.roles%"
-            $em: '@Doctrine\ORM\EntityManagerInterface'
     Chill\MainBundle\Security\Authorization\AuthorizationHelper: '@chill.main.security.authorization.helper'
 
     chill.main.role_provider:

src/Bundle/ChillTaskBundle/Security/Authorization/TaskVoter.php

@@ -18,7 +18,12 @@
 
 namespace Chill\TaskBundle\Security\Authorization;
 
+use Chill\EventBundle\Entity\Event;
+use Chill\MainBundle\Entity\Center;
 use Chill\MainBundle\Security\Authorization\AbstractChillVoter;
+use Chill\MainBundle\Security\Authorization\VoterFactoryInterface;
+use Chill\MainBundle\Security\Authorization\VoterInterface;
+use Chill\MainBundle\Security\Resolver\CenterResolverDispatcher;
 use Chill\TaskBundle\Entity\AbstractTask;
 use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
 use Chill\MainBundle\Security\Authorization\AuthorizationHelper;
@@ -32,12 +37,7 @@ use Symfony\Component\Security\Core\Role\Role;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Chill\TaskBundle\Security\Authorization\AuthorizationEvent;
 
-/**
+final class TaskVoter extends AbstractChillVoter implements ProvideRoleHierarchyInterface
- *
- *
- * @author Julien Fastré <julien.fastre@champs-libres.coop>
- */
-class TaskVoter extends AbstractChillVoter implements ProvideRoleHierarchyInterface
 {
     const CREATE = 'CHILL_TASK_TASK_CREATE';
     const UPDATE = 'CHILL_TASK_TASK_UPDATE';
@@ -51,50 +51,52 @@ class TaskVoter extends AbstractChillVoter implements ProvideRoleHierarchyInterf
         self::DELETE
     ];
 
-    /**
+    protected AuthorizationHelper $authorizationHelper;
-     *
-     * @var AuthorizationHelper
-     */
-    protected $authorizationHelper;
 
-    /**
+    protected AccessDecisionManagerInterface $accessDecisionManager;
-     *
-     * @var AccessDecisionManagerInterface
-     */
-    protected $accessDecisionManager;
 
-    /**
+    protected LoggerInterface $logger;
-     *
-     * @var LoggerInterface
-     */
-    protected $logger;
 
-    /**
+    protected EventDispatcherInterface $eventDispatcher;
-     *
+
-     * @var EventDispatcherInterface
+    protected CenterResolverDispatcher $centerResolverDispatcher;
-     */
+
-    protected $eventDispatcher;
+    protected VoterInterface $voter;
 
     public function __construct(
         AccessDecisionManagerInterface $accessDecisionManager,
         AuthorizationHelper $authorizationHelper,
         EventDispatcherInterface $eventDispatcher,
-        LoggerInterface $logger
+        LoggerInterface $logger,
+        CenterResolverDispatcher $centerResolverDispatcher,
+        VoterFactoryInterface $voterFactory
     ) {
         $this->accessDecisionManager = $accessDecisionManager;
         $this->authorizationHelper = $authorizationHelper;
         $this->eventDispatcher = $eventDispatcher;
         $this->logger = $logger;
+        $this->centerResolverDispatcher = $centerResolverDispatcher;
+
+        $this->voter = $voterFactory
+            ->generate(AbstractTask::class)
+            ->addCheckFor(AbstractTask::class, self::ROLES)
+            ->addCheckFor(Person::class, [self::SHOW])
+            ->addCheckFor(null, [self::SHOW])
+            ->build()
+            ;
     }
 
     public function supports($attribute, $subject)
     {
+        return $this->voter->supports($attribute, $subject);
+       /*
         return ($subject instanceof AbstractTask && in_array($attribute, self::ROLES))
             ||
             ($subject instanceof Person && \in_array($attribute, [ self::CREATE, self::SHOW ]))
             ||
             (NULL === $subject && $attribute === self::SHOW )
             ;
+       */
     }
 
     /**
@@ -127,6 +129,22 @@ class TaskVoter extends AbstractChillVoter implements ProvideRoleHierarchyInterf
             return $event->getVote();
         }
 
+        // do pre-flight check, relying on other decision manager
+        // those pre-flight check concern associated entities
+        if ($subject instanceof AbstractTask) {
+            if (NULL !== $person = $subject->getPerson()) {
+                if (!$this->accessDecisionManager->decide($token, [PersonVoter::SEE], $person)) {
+                    return false;
+                }
+            } elseif (false) {
+                // here will come the test if the task is associated to an accompanying course
+            }
+        }
+
+        // do regular check.
+        return $this->voter->voteOnAttribute($attribute, $subject, $token);
+
+       /*
         if ($subject instanceof AbstractTask) {
             if ($subject->getPerson() === null) {
                 throw new \LogicException("You should associate a person with task "
@@ -144,7 +162,11 @@ class TaskVoter extends AbstractChillVoter implements ProvideRoleHierarchyInterf
         }
 
         if (!$this->accessDecisionManager->decide($token, [PersonVoter::SEE], $person)) {
+            return false;
+        }
+        $center = $this->centerResolverDispatcher->resolveCenter($subject);
 
+        if (NULL === $center) {
             return false;
         }
 
@@ -153,6 +175,7 @@ class TaskVoter extends AbstractChillVoter implements ProvideRoleHierarchyInterf
             $subject,
             $attribute
             );
+       */
     }
 
     public function getRoles()

src/Bundle/ChillTaskBundle/config/services/security.yaml

@@ -1,11 +1,7 @@
 services:
     chill_task.task_voter:
         class: Chill\TaskBundle\Security\Authorization\TaskVoter
-        arguments:
+        autowire: true
-            - "@security.access.decision_manager"
+        autoconfigure: true
-            - "@chill.main.security.authorization.helper"
-            - '@Symfony\Component\EventDispatcher\EventDispatcherInterface'
-            - "@logger"
         tags:
-            - { name: security.voter }
             - { name: chill.role }