From 120f7d802600dd2769b0d22cede92cb3f92bc93d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 20 Sep 2021 13:03:59 +0200 Subject: [PATCH] upgrade voter and acl for activities and implement autoconfiguration for ChillProvideRole interface --- .../Controller/ActivityController.php | 27 +++--- .../Repository/ActivityACLAwareRepository.php | 72 +++++++++++++--- .../ActivityACLAwareRepositoryInterface.php | 19 +++++ .../Repository/ActivityRepository.php | 68 +++++++++++++-- .../Security/Authorization/ActivityVoter.php | 85 +++++++++++-------- .../ChillActivityBundle/config/services.yaml | 21 ++--- .../config/services/controller.yaml | 6 +- .../config/services/repositories.yaml | 8 +- .../ChillMainBundle/ChillMainBundle.php | 4 + .../Authorization/AuthorizationHelper.php | 21 +++-- .../Entity/AccompanyingPeriod.php | 6 +- 11 files changed, 236 insertions(+), 101 deletions(-) create mode 100644 src/Bundle/ChillActivityBundle/Repository/ActivityACLAwareRepositoryInterface.php diff --git a/src/Bundle/ChillActivityBundle/Controller/ActivityController.php b/src/Bundle/ChillActivityBundle/Controller/ActivityController.php index fe515918a..0988eedfe 100644 --- a/src/Bundle/ChillActivityBundle/Controller/ActivityController.php +++ b/src/Bundle/ChillActivityBundle/Controller/ActivityController.php @@ -22,6 +22,9 @@ namespace Chill\ActivityBundle\Controller; +use Chill\ActivityBundle\Repository\ActivityACLAwareRepository; +use Chill\ActivityBundle\Repository\ActivityACLAwareRepositoryInterface; +use Chill\ActivityBundle\Security\Authorization\ActivityVoter; use Chill\MainBundle\Security\Authorization\AuthorizationHelper; use Chill\PersonBundle\Entity\AccompanyingPeriod; use Chill\PersonBundle\Entity\Person; @@ -53,12 +56,16 @@ class ActivityController extends AbstractController protected SerializerInterface $serializer; + protected ActivityACLAwareRepositoryInterface $activityACLAwareRepository; + public function __construct( + ActivityACLAwareRepositoryInterface $activityACLAwareRepository, EventDispatcherInterface $eventDispatcher, AuthorizationHelper $authorizationHelper, LoggerInterface $logger, SerializerInterface $serializer ) { + $this->activityACLAwareRepository = $activityACLAwareRepository; $this->eventDispatcher = $eventDispatcher; $this->authorizationHelper = $authorizationHelper; $this->logger = $logger; @@ -77,13 +84,9 @@ class ActivityController extends AbstractController [$person, $accompanyingPeriod] = $this->getEntity($request); if ($person instanceof Person) { - $reachableScopes = $this->authorizationHelper - ->getReachableCircles($this->getUser(), new Role('CHILL_ACTIVITY_SEE'), - $person->getCenter()); - - $activities = $em->getRepository(Activity::class) - ->findByPersonImplied($person, $reachableScopes) - ; + $this->denyAccessUnlessGranted(ActivityVoter::SEE, $person); + $activities = $this->activityACLAwareRepository + ->findByPerson($person, ActivityVoter::SEE, 0, null); $event = new PrivacyEvent($person, array( 'element_class' => Activity::class, @@ -93,10 +96,10 @@ class ActivityController extends AbstractController $view = 'ChillActivityBundle:Activity:listPerson.html.twig'; } elseif ($accompanyingPeriod instanceof AccompanyingPeriod) { - $activities = $em->getRepository('ChillActivityBundle:Activity')->findBy( - ['accompanyingPeriod' => $accompanyingPeriod], - ['date' => 'DESC'], - ); + $this->denyAccessUnlessGranted(ActivityVoter::SEE, $accompanyingPeriod); + + $activities = $this->activityACLAwareRepository + ->findByAccompanyingPeriod($accompanyingPeriod, ActivityVoter::SEE); $view = 'ChillActivityBundle:Activity:listAccompanyingCourse.html.twig'; } @@ -238,7 +241,7 @@ class ActivityController extends AbstractController if (!$entity) { throw $this->createNotFoundException('Unable to find Activity entity.'); } - + if (null !== $accompanyingPeriod) { $entity->personsAssociated = $entity->getPersonsAssociated(); $entity->personsNotAssociated = $entity->getPersonsNotAssociated(); diff --git a/src/Bundle/ChillActivityBundle/Repository/ActivityACLAwareRepository.php b/src/Bundle/ChillActivityBundle/Repository/ActivityACLAwareRepository.php index b198875c5..b498a6090 100644 --- a/src/Bundle/ChillActivityBundle/Repository/ActivityACLAwareRepository.php +++ b/src/Bundle/ChillActivityBundle/Repository/ActivityACLAwareRepository.php @@ -23,6 +23,8 @@ namespace Chill\ActivityBundle\Repository; use Chill\ActivityBundle\Entity\Activity; +use Chill\MainBundle\Security\Resolver\CenterResolverDispatcher; +use Chill\PersonBundle\Entity\AccompanyingPeriod; use Chill\PersonBundle\Entity\Person; use Chill\ActivityBundle\Repository\ActivityRepository; use Chill\ActivityBundle\Security\Authorization\ActivityVoter; @@ -33,9 +35,10 @@ use Chill\MainBundle\Security\Authorization\AuthorizationHelper; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Role\Role; use Doctrine\ORM\EntityManagerInterface; +use Symfony\Component\Security\Core\Security; -final class ActivityACLAwareRepository +final class ActivityACLAwareRepository implements ActivityACLAwareRepositoryInterface { private AuthorizationHelper $authorizationHelper; @@ -45,16 +48,63 @@ final class ActivityACLAwareRepository private EntityManagerInterface $em; + private Security $security; + + private CenterResolverDispatcher $centerResolverDispatcher; + public function __construct( AuthorizationHelper $authorizationHelper, + CenterResolverDispatcher $centerResolverDispatcher, TokenStorageInterface $tokenStorage, ActivityRepository $repository, - EntityManagerInterface $em + EntityManagerInterface $em, + Security $security ) { $this->authorizationHelper = $authorizationHelper; + $this->centerResolverDispatcher = $centerResolverDispatcher; $this->tokenStorage = $tokenStorage; $this->repository = $repository; $this->em = $em; + $this->security = $security; + } + + /** + * @param Person $person + * @param string $role + * @param int|null $start + * @param int|null $limit + * @param array $orderBy + * @return array|Activity[] + */ + public function findByPerson(Person $person, string $role, ?int $start = 0, ?int $limit = 1000, ?array $orderBy = []): array + { + $user = $this->security->getUser(); + $center = $this->centerResolverDispatcher->resolveCenter($person); + if (0 === count($orderBy)) { + $orderBy = ['date' => 'DESC']; + } + + $reachableScopes = $this->authorizationHelper + ->getReachableCircles($user, $role, $center); + + return $this->em->getRepository(Activity::class) + ->findByPersonImplied($person, $reachableScopes, $orderBy, $limit, $start); + ; + } + + public function findByAccompanyingPeriod(AccompanyingPeriod $period, string $role, ?int $start = 0, ?int $limit = 1000, ?array $orderBy = []): array + { + $user = $this->security->getUser(); + $center = $this->centerResolverDispatcher->resolveCenter($period); + if (0 === count($orderBy)) { + $orderBy = ['date' => 'DESC']; + } + + $scopes = $this->authorizationHelper + ->getReachableCircles($user, $role, $center); + + return $this->em->getRepository(Activity::class) + ->findByAccompanyingPeriod($period, $scopes, true, $limit, $start, $orderBy); } public function queryTimelineIndexer(string $context, array $args = []): array @@ -81,7 +131,7 @@ final class ActivityACLAwareRepository $metadataActivity = $this->em->getClassMetadata(Activity::class); $metadataPerson = $this->em->getClassMetadata(Person::class); $associationMapping = $metadataActivity->getAssociationMapping('person'); - + return $metadataActivity->getTableName().' JOIN ' .$metadataPerson->getTableName().' ON ' .$metadataPerson->getTableName().'.'. @@ -95,7 +145,7 @@ final class ActivityACLAwareRepository { $where = ''; $parameters = []; - + $metadataActivity = $this->em->getClassMetadata(Activity::class); $metadataPerson = $this->em->getClassMetadata(Person::class); $activityToPerson = $metadataActivity->getAssociationMapping('person')['joinColumns'][0]['name']; @@ -105,20 +155,20 @@ final class ActivityACLAwareRepository // acls: $role = new Role(ActivityVoter::SEE); - $reachableCenters = $this->authorizationHelper->getReachableCenters($this->tokenStorage->getToken()->getUser(), + $reachableCenters = $this->authorizationHelper->getReachableCenters($this->tokenStorage->getToken()->getUser(), $role); - + if (count($reachableCenters) === 0) { // insert a dummy condition return 'FALSE = TRUE'; } - if ($context === 'person') { - // we start with activities having the person_id linked to person + if ($context === 'person') { + // we start with activities having the person_id linked to person $where .= sprintf('%s = ? AND ', $activityToPerson); $parameters[] = $person->getId(); } - + // we add acl (reachable center and scopes) $where .= '('; // first loop for the for centers $centersI = 0; // like centers#i @@ -131,7 +181,7 @@ final class ActivityACLAwareRepository $reachableScopes = $this->authorizationHelper->getReachableScopes($this->tokenStorage->getToken()->getUser(), $role, $center); // we get the ids for those scopes $reachablesScopesId = array_map( - function(Scope $scope) { return $scope->getId(); }, + function(Scope $scope) { return $scope->getId(); }, $reachableScopes ); @@ -162,7 +212,7 @@ final class ActivityACLAwareRepository } // close loop for centers $where .= ')'; - + return [$where, $parameters]; } diff --git a/src/Bundle/ChillActivityBundle/Repository/ActivityACLAwareRepositoryInterface.php b/src/Bundle/ChillActivityBundle/Repository/ActivityACLAwareRepositoryInterface.php new file mode 100644 index 000000000..0b646f7c5 --- /dev/null +++ b/src/Bundle/ChillActivityBundle/Repository/ActivityACLAwareRepositoryInterface.php @@ -0,0 +1,19 @@ + 'DESC'], $limit = 100, $offset = 0) + /** + * @param $person + * @param array $scopes + * @param string[] $orderBy + * @param int $limit + * @param int $offset + * @return array|Activity[] + */ + public function findByPersonImplied(Person $person, array $scopes, ?array $orderBy = [ 'date' => 'DESC'], ?int $limit = 100, ?int $offset = 0): array { $qb = $this->createQueryBuilder('a'); $qb->select('a'); $qb - // TODO add acl - //->where($qb->expr()->in('a.scope', ':scopes')) - //->setParameter('scopes', $scopes) + ->where($qb->expr()->in('a.scope', ':scopes')) + ->setParameter('scopes', $scopes) ->andWhere( $qb->expr()->orX( $qb->expr()->eq('a.person', ':person'), @@ -61,7 +70,56 @@ class ActivityRepository extends ServiceEntityRepository $qb->addOrderBy('a.'.$k, $dir); } + $qb->setMaxResults($limit)->setFirstResult($offset); + return $qb->getQuery() ->getResult(); - } + } + + /** + * @param AccompanyingPeriod $period + * @param array $scopes + * @param int|null $limit + * @param int|null $offset + * @param array|string[] $orderBy + * @return array|Activity[] + */ + public function findByAccompanyingPeriod(AccompanyingPeriod $period, array $scopes, ?bool $allowNullScope = false, ?int $limit = 100, ?int $offset = 0, array $orderBy = ['date' => 'desc']): array + { + $qb = $this->createQueryBuilder('a'); + $qb->select('a'); + + if (!$allowNullScope) { + $qb + ->where($qb->expr()->in('a.scope', ':scopes')) + ->setParameter('scopes', $scopes) + ; + } else { + $qb + ->where( + $qb->expr()->orX( + $qb->expr()->in('a.scope', ':scopes'), + $qb->expr()->isNull('a.scope') + ) + ) + ->setParameter('scopes', $scopes) + ; + } + + $qb + ->andWhere( + $qb->expr()->eq('a.accompanyingPeriod', ':period') + ) + ->setParameter('period', $period) + ; + + foreach ($orderBy as $k => $dir) { + $qb->addOrderBy('a.'.$k, $dir); + } + + $qb->setMaxResults($limit)->setFirstResult($offset); + + return $qb->getQuery() + ->getResult(); + } } diff --git a/src/Bundle/ChillActivityBundle/Security/Authorization/ActivityVoter.php b/src/Bundle/ChillActivityBundle/Security/Authorization/ActivityVoter.php index 79cb6d852..cc9cecf52 100644 --- a/src/Bundle/ChillActivityBundle/Security/Authorization/ActivityVoter.php +++ b/src/Bundle/ChillActivityBundle/Security/Authorization/ActivityVoter.php @@ -19,6 +19,11 @@ namespace Chill\ActivityBundle\Security\Authorization; +use Chill\MainBundle\Security\Authorization\VoterHelperFactoryInterface; +use Chill\MainBundle\Security\Authorization\VoterHelperInterface; +use Chill\PersonBundle\Entity\AccompanyingPeriod; +use Chill\PersonBundle\Security\Authorization\AccompanyingPeriodVoter; +use Chill\PersonBundle\Security\Authorization\PersonVoter; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Chill\MainBundle\Security\Authorization\AbstractChillVoter; @@ -28,11 +33,10 @@ use Chill\MainBundle\Entity\User; use Chill\ActivityBundle\Entity\Activity; use Chill\PersonBundle\Entity\Person; use Symfony\Component\Security\Core\Role\Role; +use Symfony\Component\Security\Core\Security; /** - * - * - * @author Julien Fastré + * Voter for Activity class */ class ActivityVoter extends AbstractChillVoter implements ProvideRoleHierarchyInterface { @@ -41,30 +45,37 @@ class ActivityVoter extends AbstractChillVoter implements ProvideRoleHierarchyIn const SEE_DETAILS = 'CHILL_ACTIVITY_SEE_DETAILS'; const UPDATE = 'CHILL_ACTIVITY_UPDATE'; const DELETE = 'CHILL_ACTIVITY_DELETE'; + const FULL = 'CHILL_ACTIVITY_FULL'; - /** - * - * @var AuthorizationHelper - */ - protected $helper; + private const ALL = [ + self::CREATE, + self::SEE, + self::UPDATE, + self::DELETE, + self::SEE_DETAILS, + self::FULL + ]; - public function __construct(AuthorizationHelper $helper) - { - $this->helper = $helper; + protected VoterHelperInterface $voterHelper; + + protected Security $security; + + public function __construct( + Security $security, + VoterHelperFactoryInterface $voterHelperFactory + ) { + $this->security = $security; + $this->voterHelper = $voterHelperFactory->generate(self::class) + ->addCheckFor(Person::class, [self::SEE, self::CREATE]) + ->addCheckFor(AccompanyingPeriod::class, [self::SEE, self::CREATE]) + ->addCheckFor(Activity::class, self::ALL) + ->build(); } protected function supports($attribute, $subject) { - if ($subject instanceof Activity) { - return \in_array($attribute, $this->getAttributes()); - } elseif ($subject instanceof Person) { - return $attribute === self::SEE - || - $attribute === self::CREATE; - } else { - return false; - } + return $this->voterHelper->supports($attribute, $subject); } protected function voteOnAttribute($attribute, $subject, TokenInterface $token) @@ -72,32 +83,34 @@ class ActivityVoter extends AbstractChillVoter implements ProvideRoleHierarchyIn if (!$token->getUser() instanceof User) { return false; } - - if ($subject instanceof Person) { - $centers = $this->helper->getReachableCenters($token->getUser(), new Role($attribute)); - - return \in_array($subject->getCenter(), $centers); + + if ($subject instanceof Activity) { + if ($subject->getPerson() instanceof Person) { + // the context is person: we must have the right to see the person + if (!$this->security->isGranted(PersonVoter::SEE, $subject->getPerson())) { + return false; + } + } elseif ($subject->getAccompanyingPeriod() instanceof AccompanyingPeriod) { + if (!$this->security->isGranted(AccompanyingPeriodVoter::SEE, $subject->getAccompanyingPeriod())) { + return false; + } + } else { + throw new \RuntimeException("could not determine context of activity"); + } } - - /* @var $subject Activity */ - return $this->helper->userHasAccess($token->getUser(), $subject, $attribute); - } - - private function getAttributes() - { - return [ self::CREATE, self::SEE, self::UPDATE, self::DELETE, - self::SEE_DETAILS ]; + + return $this->voterHelper->voteOnAttribute($attribute, $subject, $token); } public function getRoles() { - return $this->getAttributes(); + return self::ALL; } public function getRolesWithoutScope() { - return array(); + return []; } diff --git a/src/Bundle/ChillActivityBundle/config/services.yaml b/src/Bundle/ChillActivityBundle/config/services.yaml index 86168101a..0a65e08c8 100644 --- a/src/Bundle/ChillActivityBundle/config/services.yaml +++ b/src/Bundle/ChillActivityBundle/config/services.yaml @@ -1,20 +1,4 @@ services: - chill.activity.security.authorization.activity_voter: - class: Chill\ActivityBundle\Security\Authorization\ActivityVoter - arguments: - - "@chill.main.security.authorization.helper" - tags: - - { name: security.voter } - - { name: chill.role } - - chill.activity.security.authorization.activity_stats_voter: - class: Chill\ActivityBundle\Security\Authorization\ActivityStatsVoter - arguments: - - "@chill.main.security.authorization.helper" - tags: - - { name: security.voter } - - { name: chill.role } - chill.activity.timeline: class: Chill\ActivityBundle\Timeline\TimelineActivityProvider @@ -38,3 +22,8 @@ services: autowire: true autoconfigure: true resource: '../Notification' + + Chill\ActivityBundle\Security\Authorization\: + resource: '../Security/Authorization/' + autowire: true + autoconfigure: true diff --git a/src/Bundle/ChillActivityBundle/config/services/controller.yaml b/src/Bundle/ChillActivityBundle/config/services/controller.yaml index 106b2c6e4..96ace0a64 100644 --- a/src/Bundle/ChillActivityBundle/config/services/controller.yaml +++ b/src/Bundle/ChillActivityBundle/config/services/controller.yaml @@ -1,8 +1,4 @@ services: Chill\ActivityBundle\Controller\ActivityController: - arguments: - $eventDispatcher: '@Symfony\Component\EventDispatcher\EventDispatcherInterface' - $authorizationHelper: '@Chill\MainBundle\Security\Authorization\AuthorizationHelper' - $logger: '@chill.main.logger' - $serializer: '@Symfony\Component\Serializer\SerializerInterface' + autowire: true tags: ['controller.service_arguments'] diff --git a/src/Bundle/ChillActivityBundle/config/services/repositories.yaml b/src/Bundle/ChillActivityBundle/config/services/repositories.yaml index 2f0a9b83c..26bc58ac9 100644 --- a/src/Bundle/ChillActivityBundle/config/services/repositories.yaml +++ b/src/Bundle/ChillActivityBundle/config/services/repositories.yaml @@ -24,9 +24,7 @@ services: - '@Doctrine\Persistence\ManagerRegistry' Chill\ActivityBundle\Repository\ActivityACLAwareRepository: - arguments: - $tokenStorage: '@Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface' - $authorizationHelper: '@Chill\MainBundle\Security\Authorization\AuthorizationHelper' - $repository: '@Chill\ActivityBundle\Repository\ActivityRepository' - $em: '@Doctrine\ORM\EntityManagerInterface' + autowire: true + autoconfigure: true + Chill\ActivityBundle\Repository\ActivityACLAwareRepositoryInterface: '@Chill\ActivityBundle\Repository\ActivityACLAwareRepository' diff --git a/src/Bundle/ChillMainBundle/ChillMainBundle.php b/src/Bundle/ChillMainBundle/ChillMainBundle.php index d5fb7b79d..91c58f68b 100644 --- a/src/Bundle/ChillMainBundle/ChillMainBundle.php +++ b/src/Bundle/ChillMainBundle/ChillMainBundle.php @@ -4,6 +4,8 @@ namespace Chill\MainBundle; use Chill\MainBundle\Routing\LocalMenuBuilderInterface; use Chill\MainBundle\Search\SearchInterface; +use Chill\MainBundle\Security\Authorization\ChillVoterInterface; +use Chill\MainBundle\Security\ProvideRoleInterface; use Chill\MainBundle\Security\Resolver\CenterResolverInterface; use Chill\MainBundle\Security\Resolver\ScopeResolverInterface; use Symfony\Component\HttpKernel\Bundle\Bundle; @@ -30,6 +32,8 @@ class ChillMainBundle extends Bundle $container->registerForAutoconfiguration(LocalMenuBuilderInterface::class) ->addTag('chill.menu_builder'); + $container->registerForAutoconfiguration(ProvideRoleInterface::class) + ->addTag('chill.role'); $container->registerForAutoconfiguration(CenterResolverInterface::class) ->addTag('chill_main.center_resolver'); $container->registerForAutoconfiguration(ScopeResolverInterface::class) diff --git a/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php b/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php index bf45a98d6..2b223a0fe 100644 --- a/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php +++ b/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php @@ -265,11 +265,11 @@ class AuthorizationHelper * @deprecated Use getReachableCircles * * @param User $user - * @param Role $role - * @param Center $center + * @param string role + * @param Center|Center[] $center * @return Scope[] */ - public function getReachableScopes(User $user, $role, Center $center) + public function getReachableScopes(User $user, $role, $center) { if ($role instanceof Role) { $role = $role->getRole(); @@ -283,15 +283,24 @@ class AuthorizationHelper * * @param User $user * @param string|Role $role - * @param Center $center + * @param Center|Center[] $center * @return Scope[] */ - public function getReachableCircles(User $user, $role, Center $center) + public function getReachableCircles(User $user, $role, $center) { + $scopes = []; + + if (is_iterable($center)) { + foreach ($center as $c) { + $scopes = \array_merge($scopes, $this->getReachableCircles($user, $role, $c)); + } + + return $scopes; + } + if ($role instanceof Role) { $role = $role->getRole(); } - $scopes = array(); foreach ($user->getGroupCenters() as $groupCenter){ if ($center->getId() === $groupCenter->getCenter()->getId()) { diff --git a/src/Bundle/ChillPersonBundle/Entity/AccompanyingPeriod.php b/src/Bundle/ChillPersonBundle/Entity/AccompanyingPeriod.php index 5672b7f09..d69a7a51a 100644 --- a/src/Bundle/ChillPersonBundle/Entity/AccompanyingPeriod.php +++ b/src/Bundle/ChillPersonBundle/Entity/AccompanyingPeriod.php @@ -1050,7 +1050,7 @@ class AccompanyingPeriod implements TrackCreationInterface, TrackUpdateInterface } public function getCenters(): ?iterable - {dump(__METHOD__); + { foreach ($this->getPersons() as $person) { if (!in_array($person->getCenter(), $centers ?? []) && NULL !== $person->getCenter()) { @@ -1058,10 +1058,6 @@ class AccompanyingPeriod implements TrackCreationInterface, TrackUpdateInterface } } - dump($centers); - return $centers ?? null; } - - }