Merge branch 'bugfix/acl-add-roles-misc-places' into 'master'

Add voter to various places

See merge request Chill-Projet/chill-bundles!207

CHANGELOG.md

@@ -24,6 +24,8 @@ and this project adheres to
 * refactor `AuthorizationHelper` and `UserACLAwareRepository` to fix constructor, and separate logic for parent role helper into `ParentRoleHelper`
 * [main]: filter location and locationType in backend: exclude NULL names, only active and availableToUsers
 * [activity]: perform client-side validation & show/hide fields in the "new location" modal
+* [docstore] voter for PersonDocument and AccompanyingCourseDocument on the 2.0 way (using VoterHelperFactory)
+* [docstore] add authorization check inside controller and menu
 * [activity]: fix inheritance for role `ACTIVITY FULL` and add missing acl in menu
 * [person] show current address in search results
 * [person] show alt names in search results

src/Bundle/ChillDocStoreBundle/Controller/DocumentAccompanyingCourseController.php

@@ -4,6 +4,7 @@ namespace Chill\DocStoreBundle\Controller;
 
 use Chill\DocStoreBundle\Entity\AccompanyingCourseDocument;
 use Chill\DocStoreBundle\Form\AccompanyingCourseDocumentType;
+use Chill\DocStoreBundle\Security\Authorization\AccompanyingCourseDocumentVoter;
 use Chill\MainBundle\Security\Authorization\AuthorizationHelper;
 use Chill\PersonBundle\Entity\AccompanyingPeriod;
 use Chill\PersonBundle\Privacy\PrivacyEvent;
@@ -16,12 +17,7 @@ use Symfony\Contracts\Translation\TranslatorInterface;
 use Symfony\Component\Routing\Annotation\Route;
 
 /**
- * Class DocumentAccompanyingCourseController
- *
- * @package Chill\DocStoreBundle\Controller
  * @Route("/{_locale}/parcours/{course}/document")
- *
- * TODO faire un controller abstrait ?
  */
 class DocumentAccompanyingCourseController extends AbstractController
 {
@@ -70,7 +66,7 @@ class DocumentAccompanyingCourseController extends AbstractController
             throw $this->createNotFoundException('Accompanying period not found');
         }
 
-        $this->denyAccessUnlessGranted(AccompanyingPeriodVoter::SEE, $course);
+        $this->denyAccessUnlessGranted(AccompanyingCourseDocumentVoter::SEE, $course);
 
         $documents = $em
             ->getRepository("ChillDocStoreBundle:AccompanyingCourseDocument")
@@ -96,13 +92,13 @@ class DocumentAccompanyingCourseController extends AbstractController
             throw $this->createNotFoundException('Accompanying period not found');
         }
 
-        $this->denyAccessUnlessGranted(AccompanyingPeriodVoter::SEE, $course);
-
         $document = new AccompanyingCourseDocument();
         $document->setUser($this->getUser());
         $document->setCourse($course);
         $document->setDate(new \DateTime('Now'));
 
+        $this->denyAccessUnlessGranted(AccompanyingCourseDocumentVoter::CREATE, $document);
+
         $form = $this->createForm(AccompanyingCourseDocumentType::class, $document);
         $form->handleRequest($request);
 
@@ -134,8 +130,7 @@ class DocumentAccompanyingCourseController extends AbstractController
      */
     public function show(AccompanyingPeriod $course, AccompanyingCourseDocument $document): Response
     {
-        $this->denyAccessUnlessGranted('CHILL_PERSON_ACCOMPANYING_PERIOD_SEE', $course);
+        $this->denyAccessUnlessGranted(AccompanyingCourseDocumentVoter::SEE_DETAILS, $document);
-        $this->denyAccessUnlessGranted('CHILL_ACCOMPANYING_COURSE_DOCUMENT_SEE', $document);
 
         return $this->render(
             'ChillDocStoreBundle:AccompanyingCourseDocument:show.html.twig',
@@ -147,8 +142,7 @@ class DocumentAccompanyingCourseController extends AbstractController
      */
     public function edit(Request $request, AccompanyingPeriod $course, AccompanyingCourseDocument $document): Response
     {
-        $this->denyAccessUnlessGranted('CHILL_PERSON_ACCOMPANYING_PERIOD_SEE', $course);
+        $this->denyAccessUnlessGranted(AccompanyingCourseDocumentVoter::UPDATE, $document);
-        $this->denyAccessUnlessGranted('CHILL_ACCOMPANYING_COURSE_DOCUMENT_UPDATE', $document);
 
         $document->setUser($this->getUser());
         $document->setDate(new \DateTime('Now'));
@@ -184,8 +178,7 @@ class DocumentAccompanyingCourseController extends AbstractController
      */
     public function delete(Request $request, AccompanyingPeriod $course, AccompanyingCourseDocument $document): Response
     {
-        $this->denyAccessUnlessGranted('CHILL_PERSON_ACCOMPANYING_PERIOD_SEE', $course);
+        $this->denyAccessUnlessGranted(AccompanyingCourseDocumentVoter::DELETE, $document);
-        $this->denyAccessUnlessGranted('CHILL_ACCOMPANYING_COURSE_DOCUMENT_DELETE', $document);
 
         if ($this->isCsrfTokenValid('delete'.$document->getId(), $request->request->get('_token'))) {
             $em = $this->getDoctrine()->getManager();

src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php

@@ -2,6 +2,7 @@
 
 namespace Chill\DocStoreBundle\DependencyInjection;
 
+use Chill\DocStoreBundle\Security\Authorization\AccompanyingCourseDocumentVoter;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\HttpKernel\DependencyInjection\Extension;
@@ -61,6 +62,10 @@ class ChillDocStoreExtension extends Extension implements PrependExtensionInterf
                PersonDocumentVoter::CREATE => [PersonDocumentVoter::SEE_DETAILS],
                PersonDocumentVoter::DELETE => [PersonDocumentVoter::SEE_DETAILS],
                PersonDocumentVoter::SEE_DETAILS => [PersonDocumentVoter::SEE],
+               AccompanyingCourseDocumentVoter::UPDATE => [AccompanyingCourseDocumentVoter::SEE_DETAILS],
+               AccompanyingCourseDocumentVoter::CREATE => [AccompanyingCourseDocumentVoter::SEE_DETAILS],
+               AccompanyingCourseDocumentVoter::DELETE => [AccompanyingCourseDocumentVoter::SEE_DETAILS],
+               AccompanyingCourseDocumentVoter::SEE_DETAILS => [AccompanyingCourseDocumentVoter::SEE],
            )
         ));
     }

src/Bundle/ChillDocStoreBundle/Menu/MenuBuilder.php

@@ -3,46 +3,23 @@
  */
 namespace Chill\DocStoreBundle\Menu;
 
+use Chill\DocStoreBundle\Security\Authorization\AccompanyingCourseDocumentVoter;
 use Chill\MainBundle\Routing\LocalMenuBuilderInterface;
-use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
 use Knp\Menu\MenuItem;
-use Chill\MainBundle\Security\Authorization\AuthorizationHelper;
 use Chill\DocStoreBundle\Security\Authorization\PersonDocumentVoter;
-use Symfony\Component\Security\Core\Role\Role;
+use Symfony\Component\Security\Core\Security;
-use Symfony\Component\Translation\TranslatorInterface;
+use Symfony\Contracts\Translation\TranslatorInterface;
 
-/**
+final class MenuBuilder implements LocalMenuBuilderInterface
- * 
- *
- * @author Julien Fastré <julien.fastre@champs-libres.coop>
- */
-class MenuBuilder implements LocalMenuBuilderInterface
 {
-    /**
+    private Security $security;
-     *
+    protected TranslatorInterface $translator;
-     * @var TokenStorageInterface
-     */
-    protected $tokenStorage;
-    
-    /**
-     *
-     * @var AuthorizationHelper
-     */
-    protected $authorizationHelper;
-    
-    /**
-     *
-     * @var TranslatorInterface
-     */
-    protected $translator;
 
     public function __construct(
-        TokenStorageInterface $tokenStorage, 
+        Security $security,
-        AuthorizationHelper $authorizationHelper, 
         TranslatorInterface $translator
-    ){
+    ) {
-        $this->tokenStorage = $tokenStorage;
+        $this->security = $security;
-        $this->authorizationHelper = $authorizationHelper;
         $this->translator = $translator;
     }
 
@@ -65,11 +42,8 @@ class MenuBuilder implements LocalMenuBuilderInterface
     {
         /* @var $person \Chill\PersonBundle\Entity\Person */
         $person = $parameters['person'];
-        $user = $this->tokenStorage->getToken()->getUser();
-        
-        if ($this->authorizationHelper->userHasAccess($user, 
-            $person->getCenter(), PersonDocumentVoter::SEE)) {
 
+        if ($this->security->isGranted(PersonDocumentVoter::SEE, $person)) {
             $menu->addChild($this->translator->trans('Documents'),  [
                     'route' => 'person_document_index',
                     'routeParameters' => [
@@ -80,24 +54,22 @@ class MenuBuilder implements LocalMenuBuilderInterface
                     'order'=> 350
                 ]);
         }
-        
     }
 
     protected function buildMenuAccompanyingCourse(MenuItem $menu, array $parameters){
         $course = $parameters['accompanyingCourse'];
-        // $user = $this->tokenStorage->getToken()->getUser();
 
-        //TODO : add condition to check user rights?
+        if ($this->security->isGranted(AccompanyingCourseDocumentVoter::SEE, $course)) {
-
+            $menu->addChild($this->translator->trans('Documents'), [
-        $menu->addChild($this->translator->trans('Documents'),  [
                 'route' => 'accompanying_course_document_index',
                 'routeParameters' => [
                     'course' => $course->getId()
                 ]
             ])
-            ->setExtras([
+                ->setExtras([
-                'order'=> 400
+                    'order' => 400
-            ]);
+                ]);
+        }
     }
 
     public static function getMenuIds(): array

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

@@ -3,13 +3,20 @@
 namespace Chill\DocStoreBundle\Security\Authorization;
 
 use Chill\DocStoreBundle\Entity\AccompanyingCourseDocument;
+use Chill\DocStoreBundle\Entity\PersonDocument;
 use Chill\MainBundle\Security\Authorization\AbstractChillVoter;
 use Chill\MainBundle\Security\Authorization\AuthorizationHelper;
+use Chill\MainBundle\Security\Authorization\VoterHelperFactoryInterface;
+use Chill\MainBundle\Security\Authorization\VoterHelperInterface;
 use Chill\MainBundle\Security\ProvideRoleHierarchyInterface;
 use Chill\MainBundle\Entity\User;
+use Chill\PersonBundle\Entity\AccompanyingPeriod;
+use Chill\PersonBundle\Entity\Person;
+use Chill\PersonBundle\Security\Authorization\AccompanyingPeriodVoter;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
 use Psr\Log\LoggerInterface;
+use Symfony\Component\Security\Core\Security;
 
 /**
  *
@@ -22,30 +29,22 @@ class AccompanyingCourseDocumentVoter extends AbstractChillVoter implements Prov
     const UPDATE = 'CHILL_ACCOMPANYING_COURSE_DOCUMENT_UPDATE';
     const DELETE = 'CHILL_ACCOMPANYING_COURSE_DOCUMENT_DELETE';
 
-    /**
+    protected LoggerInterface $logger;
-     * @var AuthorizationHelper
+    protected VoterHelperInterface $voterHelper;
-     */
+    protected Security $security;
-    protected $authorizationHelper;
-
-    /**
-     * @var AccessDecisionManagerInterface
-     */
-    protected $accessDecisionManager;
-
-    /**
-     * @var LoggerInterface
-     */
-    protected $logger;
 
     public function __construct(
-        AccessDecisionManagerInterface $accessDecisionManager,
+        LoggerInterface $logger,
-        AuthorizationHelper $authorizationHelper,
+        Security $security,
-        LoggerInterface $logger
+        VoterHelperFactoryInterface $voterHelperFactory
-    )
+    ) {
-    {
-        $this->accessDecisionManager = $accessDecisionManager;
-        $this->authorizationHelper = $authorizationHelper;
         $this->logger = $logger;
+        $this->security = $security;
+        $this->voterHelper = $voterHelperFactory
+            ->generate(self::class)
+            ->addCheckFor(AccompanyingCourseDocument::class, $this->getRoles())
+            ->addCheckFor(AccompanyingPeriod::class, [self::SEE, self::CREATE])
+            ->build();
     }
 
     public function getRoles()
@@ -61,26 +60,30 @@ class AccompanyingCourseDocumentVoter extends AbstractChillVoter implements Prov
 
     protected function supports($attribute, $subject)
     {
-
+        return $this->voterHelper->supports($attribute, $subject);
-        if (\in_array($attribute, $this->getRoles())) {
-            return true;
-        }
-
-        return false;
     }
 
     protected function voteOnAttribute($attribute, $subject, TokenInterface $token)
     {
-        return true;
+        $this->logger->debug(sprintf("Voting from %s class", self::class));
-    }
 
+        if (!$token->getUser() instanceof User) {
+            return false;
+        }
+
+        if ($subject instanceof AccompanyingCourseDocument
+            && !$this->security->isGranted(AccompanyingPeriodVoter::SEE, $subject->getCourse())) {
+            return false;
+        }
+
+        return $this->voterHelper->voteOnAttribute($attribute, $subject, $token);
+    }
 
     public function getRolesWithoutScope()
     {
         return array();
     }
 
-
     public function getRolesWithHierarchy()
     {
         return ['accompanyingCourseDocument' => $this->getRoles() ];

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

@@ -19,8 +19,11 @@
 
 namespace Chill\DocStoreBundle\Security\Authorization;
 
+use App\Security\Authorization\VoterHelperFactory;
 use Chill\MainBundle\Security\Authorization\AbstractChillVoter;
 use Chill\MainBundle\Security\Authorization\AuthorizationHelper;
+use Chill\MainBundle\Security\Authorization\VoterHelperFactoryInterface;
+use Chill\MainBundle\Security\Authorization\VoterHelperInterface;
 use Chill\MainBundle\Security\ProvideRoleHierarchyInterface;
 use Chill\DocStoreBundle\Entity\PersonDocument;
 use Chill\MainBundle\Security\Resolver\CenterResolverDispatcher;
@@ -31,6 +34,7 @@ use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Role\Role;
 use Psr\Log\LoggerInterface;
+use Symfony\Component\Security\Core\Security;
 
 /**
  *
@@ -43,25 +47,22 @@ class PersonDocumentVoter extends AbstractChillVoter implements ProvideRoleHiera
     const UPDATE = 'CHILL_PERSON_DOCUMENT_UPDATE';
     const DELETE = 'CHILL_PERSON_DOCUMENT_DELETE';
 
-    protected AuthorizationHelper $authorizationHelper;
-
-    protected AccessDecisionManagerInterface $accessDecisionManager;
-
     protected LoggerInterface $logger;
-
+    protected Security $security;
-    protected CenterResolverDispatcher $centerResolverDispatcher;
+    protected VoterHelperInterface $voterHelper;
 
     public function __construct(
-        AccessDecisionManagerInterface $accessDecisionManager,
+        LoggerInterface $logger,
-        AuthorizationHelper $authorizationHelper,
+        Security $security,
-        LoggerInterface $logger//,
+        VoterHelperFactoryInterface $voterHelperFactory
-        //CenterResolverDispatcher $centerResolverDispatcher
+    ) {
-    )
-    {
-        $this->accessDecisionManager = $accessDecisionManager;
-        $this->authorizationHelper = $authorizationHelper;
         $this->logger = $logger;
-        //$this->centerResolverDispatcher = $centerResolverDispatcher;
+        $this->security = $security;
+        $this->voterHelper = $voterHelperFactory
+            ->generate(self::class)
+            ->addCheckFor(PersonDocument::class, $this->getRoles())
+            ->addCheckFor(Person::class, [self::SEE, self::CREATE])
+            ->build();
     }
 
     public function getRoles()
@@ -77,16 +78,7 @@ class PersonDocumentVoter extends AbstractChillVoter implements ProvideRoleHiera
 
     protected function supports($attribute, $subject)
     {
-        if (\in_array($attribute, $this->getRoles()) && $subject instanceof PersonDocument) {
+        return $this->voterHelper->supports($attribute, $subject);
-            return true;
-        }
-
-        if ($subject instanceof Person
-            && \in_array($attribute, [self::CREATE, self::SEE])) {
-            return true;
-        }
-
-        return false;
     }
 
     /**
@@ -104,42 +96,12 @@ class PersonDocumentVoter extends AbstractChillVoter implements ProvideRoleHiera
             return false;
         }
 
-        $center = $this->centerResolverDispatcher->resolveCenter($subject);
+        if ($subject instanceof PersonDocument
-
+            && !$this->security->isGranted(PersonVoter::SEE, $subject->getPerson())) {
-        if ($subject instanceof PersonDocument) {
-            return $this->authorizationHelper->userHasAccess($token->getUser(), $subject, $attribute);
-
-        } elseif ($subject instanceof Person) {
-            return $this->authorizationHelper->userHasAccess($token->getUser(), $subject, $attribute);
-
-        } else {
-
-            // subject is null. We check that at least one center is reachable
-            $centers = $this->authorizationHelper
-                ->getReachableCenters($token->getUser(), new Role($attribute));
-
-            return count($centers) > 0;
-        }
-
-        if (!$this->accessDecisionManager->decide($token, [PersonVoter::SEE], $person)) {
             return false;
         }
 
-        return $this->authorizationHelper->userHasAccess(
+        return $this->voterHelper->voteOnAttribute($attribute, $subject, $token);
-            $token->getUser(),
-            $subject,
-            $attribute
-        );
-
-    }
-
-    protected function isGranted($attribute, $report, $user = null)
-    {
-        if (! $user instanceof User){
-            return false;
-        }
-
-        return $this->helper->userHasAccess($user, $report, $attribute);
     }
 
     public function getRolesWithoutScope()

src/Bundle/ChillDocStoreBundle/config/services/menu.yaml

@@ -1,8 +1,4 @@
 services:
     Chill\DocStoreBundle\Menu\MenuBuilder:
-        arguments:
+        autowire: true
-            $tokenStorage: '@Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface'
+        autoconfigure: true
-            $authorizationHelper: '@Chill\MainBundle\Security\Authorization\AuthorizationHelper'
-            $translator: '@Symfony\Component\Translation\TranslatorInterface'
-        tags:
-            - { name: 'chill.menu_builder' }