diff --git a/CHANGELOG.md b/CHANGELOG.md index 41b133bba..518fcabb9 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/Bundle/ChillDocStoreBundle/Controller/DocumentAccompanyingCourseController.php b/src/Bundle/ChillDocStoreBundle/Controller/DocumentAccompanyingCourseController.php index 512747b01..e0bcceef1 100644 --- a/src/Bundle/ChillDocStoreBundle/Controller/DocumentAccompanyingCourseController.php +++ b/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,32 +17,27 @@ 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 { - + /** * * @var TranslatorInterface */ protected $translator; - + /** * @var EventDispatcherInterface */ protected $eventDispatcher; - + /** * @var AuthorizationHelper */ protected $authorizationHelper; - + /** * DocumentAccompanyingCourseController constructor. @@ -50,15 +46,15 @@ class DocumentAccompanyingCourseController extends AbstractController * @param AuthorizationHelper $authorizationHelper */ public function __construct( - TranslatorInterface $translator, - EventDispatcherInterface $eventDispatcher, + TranslatorInterface $translator, + EventDispatcherInterface $eventDispatcher, AuthorizationHelper $authorizationHelper ) { $this->translator = $translator; $this->eventDispatcher = $eventDispatcher; $this->authorizationHelper = $authorizationHelper; } - + /** * @Route("/", name="accompanying_course_document_index", methods="GET") */ @@ -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") @@ -78,7 +74,7 @@ class DocumentAccompanyingCourseController extends AbstractController ['course' => $course], ['date' => 'DESC'] ); - + return $this->render( 'ChillDocStoreBundle:AccompanyingCourseDocument:index.html.twig', [ @@ -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); @@ -114,7 +110,7 @@ class DocumentAccompanyingCourseController extends AbstractController $em = $this->getDoctrine()->getManager(); $em->persist($document); $em->flush(); - + $this->addFlash('success', $this->translator->trans("The document is successfully registered")); return $this->redirectToRoute('accompanying_course_document_index', ['course' => $course->getId()]); @@ -134,9 +130,8 @@ class DocumentAccompanyingCourseController extends AbstractController */ public function show(AccompanyingPeriod $course, AccompanyingCourseDocument $document): Response { - $this->denyAccessUnlessGranted('CHILL_PERSON_ACCOMPANYING_PERIOD_SEE', $course); - $this->denyAccessUnlessGranted('CHILL_ACCOMPANYING_COURSE_DOCUMENT_SEE', $document); - + $this->denyAccessUnlessGranted(AccompanyingCourseDocumentVoter::SEE_DETAILS, $document); + return $this->render( 'ChillDocStoreBundle:AccompanyingCourseDocument:show.html.twig', ['document' => $document, 'accompanyingCourse' => $course]); @@ -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('CHILL_ACCOMPANYING_COURSE_DOCUMENT_UPDATE', $document); + $this->denyAccessUnlessGranted(AccompanyingCourseDocumentVoter::UPDATE, $document); $document->setUser($this->getUser()); $document->setDate(new \DateTime('Now')); @@ -159,17 +153,17 @@ class DocumentAccompanyingCourseController extends AbstractController if ($form->isSubmitted() && $form->isValid()) { $this->getDoctrine()->getManager()->flush(); - + $this->addFlash('success', $this->translator->trans("The document is successfully updated")); - + return $this->redirectToRoute( 'accompanying_course_document_edit', ['id' => $document->getId(), 'course' => $course->getId()]); - + } elseif ($form->isSubmitted() and !$form->isValid()) { $this->addFlash('error', $this->translator->trans("This form contains errors")); } - + return $this->render( 'ChillDocStoreBundle:AccompanyingCourseDocument:edit.html.twig', [ @@ -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('CHILL_ACCOMPANYING_COURSE_DOCUMENT_DELETE', $document); + $this->denyAccessUnlessGranted(AccompanyingCourseDocumentVoter::DELETE, $document); if ($this->isCsrfTokenValid('delete'.$document->getId(), $request->request->get('_token'))) { $em = $this->getDoctrine()->getManager(); diff --git a/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php b/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php index 63de15f8b..eac97b86e 100644 --- a/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php +++ b/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; @@ -39,7 +40,7 @@ class ChillDocStoreExtension extends Extension implements PrependExtensionInterf $this->prependAuthorization($container); $this->prependTwig($container); } - + protected function prependRoute(ContainerBuilder $container) { //declare routes for task bundle @@ -52,7 +53,7 @@ class ChillDocStoreExtension extends Extension implements PrependExtensionInterf ) )); } - + protected function prependAuthorization(ContainerBuilder $container) { $container->prependExtensionConfig('security', array( @@ -61,10 +62,14 @@ 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], ) )); } - + protected function prependTwig(ContainerBuilder $container) { $twigConfig = array( diff --git a/src/Bundle/ChillDocStoreBundle/Menu/MenuBuilder.php b/src/Bundle/ChillDocStoreBundle/Menu/MenuBuilder.php index c48d45833..4040fdc09 100644 --- a/src/Bundle/ChillDocStoreBundle/Menu/MenuBuilder.php +++ b/src/Bundle/ChillDocStoreBundle/Menu/MenuBuilder.php @@ -3,50 +3,27 @@ */ 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\Translation\TranslatorInterface; +use Symfony\Component\Security\Core\Security; +use Symfony\Contracts\Translation\TranslatorInterface; -/** - * - * - * @author Julien Fastré - */ -class MenuBuilder implements LocalMenuBuilderInterface +final class MenuBuilder implements LocalMenuBuilderInterface { - /** - * - * @var TokenStorageInterface - */ - protected $tokenStorage; - - /** - * - * @var AuthorizationHelper - */ - protected $authorizationHelper; - - /** - * - * @var TranslatorInterface - */ - protected $translator; - + private Security $security; + protected TranslatorInterface $translator; + public function __construct( - TokenStorageInterface $tokenStorage, - AuthorizationHelper $authorizationHelper, + Security $security, TranslatorInterface $translator - ){ - $this->tokenStorage = $tokenStorage; - $this->authorizationHelper = $authorizationHelper; + ) { + $this->security = $security; $this->translator = $translator; } - + public function buildMenu($menuId, MenuItem $menu, array $parameters) { switch($menuId) { @@ -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? - - $menu->addChild($this->translator->trans('Documents'), [ + if ($this->security->isGranted(AccompanyingCourseDocumentVoter::SEE, $course)) { + $menu->addChild($this->translator->trans('Documents'), [ 'route' => 'accompanying_course_document_index', 'routeParameters' => [ 'course' => $course->getId() ] ]) - ->setExtras([ - 'order'=> 400 - ]); + ->setExtras([ + 'order' => 400 + ]); + } } public static function getMenuIds(): array diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php index 944a27b27..53507d573 100644 --- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php +++ b/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'; - /** - * @var AuthorizationHelper - */ - protected $authorizationHelper; - - /** - * @var AccessDecisionManagerInterface - */ - protected $accessDecisionManager; - - /** - * @var LoggerInterface - */ - protected $logger; + protected LoggerInterface $logger; + protected VoterHelperInterface $voterHelper; + protected Security $security; public function __construct( - AccessDecisionManagerInterface $accessDecisionManager, - AuthorizationHelper $authorizationHelper, - LoggerInterface $logger - ) - { - $this->accessDecisionManager = $accessDecisionManager; - $this->authorizationHelper = $authorizationHelper; + LoggerInterface $logger, + Security $security, + VoterHelperFactoryInterface $voterHelperFactory + ) { $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) { - - if (\in_array($attribute, $this->getRoles())) { - return true; - } - - return false; + return $this->voterHelper->supports($attribute, $subject); } 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() ]; diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/PersonDocumentVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/PersonDocumentVoter.php index c92971b8f..3eb21ed19 100644 --- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/PersonDocumentVoter.php +++ b/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 CenterResolverDispatcher $centerResolverDispatcher; + protected Security $security; + protected VoterHelperInterface $voterHelper; public function __construct( - AccessDecisionManagerInterface $accessDecisionManager, - AuthorizationHelper $authorizationHelper, - LoggerInterface $logger//, - //CenterResolverDispatcher $centerResolverDispatcher - ) - { - $this->accessDecisionManager = $accessDecisionManager; - $this->authorizationHelper = $authorizationHelper; + LoggerInterface $logger, + Security $security, + VoterHelperFactoryInterface $voterHelperFactory + ) { $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 true; - } - - if ($subject instanceof Person - && \in_array($attribute, [self::CREATE, self::SEE])) { - return true; - } - - return false; + return $this->voterHelper->supports($attribute, $subject); } /** @@ -104,42 +96,12 @@ 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); - - } 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)) { + if ($subject instanceof PersonDocument + && !$this->security->isGranted(PersonVoter::SEE, $subject->getPerson())) { return false; } - return $this->authorizationHelper->userHasAccess( - $token->getUser(), - $subject, - $attribute - ); - - } - - protected function isGranted($attribute, $report, $user = null) - { - if (! $user instanceof User){ - return false; - } - - return $this->helper->userHasAccess($user, $report, $attribute); + return $this->voterHelper->voteOnAttribute($attribute, $subject, $token); } public function getRolesWithoutScope() diff --git a/src/Bundle/ChillDocStoreBundle/config/services/menu.yaml b/src/Bundle/ChillDocStoreBundle/config/services/menu.yaml index 069cd64d4..db16aef77 100644 --- a/src/Bundle/ChillDocStoreBundle/config/services/menu.yaml +++ b/src/Bundle/ChillDocStoreBundle/config/services/menu.yaml @@ -1,8 +1,4 @@ services: Chill\DocStoreBundle\Menu\MenuBuilder: - arguments: - $tokenStorage: '@Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface' - $authorizationHelper: '@Chill\MainBundle\Security\Authorization\AuthorizationHelper' - $translator: '@Symfony\Component\Translation\TranslatorInterface' - tags: - - { name: 'chill.menu_builder' } + autowire: true + autoconfigure: true