diff --git a/src/Bundle/ChillMainBundle/ChillMainBundle.php b/src/Bundle/ChillMainBundle/ChillMainBundle.php index d2ae5f1b5..d5fb7b79d 100644 --- a/src/Bundle/ChillMainBundle/ChillMainBundle.php +++ b/src/Bundle/ChillMainBundle/ChillMainBundle.php @@ -5,6 +5,7 @@ namespace Chill\MainBundle; use Chill\MainBundle\Routing\LocalMenuBuilderInterface; use Chill\MainBundle\Search\SearchInterface; use Chill\MainBundle\Security\Resolver\CenterResolverInterface; +use Chill\MainBundle\Security\Resolver\ScopeResolverInterface; use Symfony\Component\HttpKernel\Bundle\Bundle; use Symfony\Component\DependencyInjection\ContainerBuilder; use Chill\MainBundle\DependencyInjection\CompilerPass\SearchableServicesCompilerPass; @@ -31,6 +32,8 @@ class ChillMainBundle extends Bundle ->addTag('chill.menu_builder'); $container->registerForAutoconfiguration(CenterResolverInterface::class) ->addTag('chill_main.center_resolver'); + $container->registerForAutoconfiguration(ScopeResolverInterface::class) + ->addTag('chill_main.scope_resolver'); $container->addCompilerPass(new SearchableServicesCompilerPass()); $container->addCompilerPass(new ConfigConsistencyCompilerPass()); diff --git a/src/Bundle/ChillMainBundle/Entity/HasCentersInterface.php b/src/Bundle/ChillMainBundle/Entity/HasCentersInterface.php new file mode 100644 index 000000000..921cbba41 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Entity/HasCentersInterface.php @@ -0,0 +1,8 @@ +roleHierarchy = $roleHierarchy; $this->hierarchy = $parameterBag->get('security.role_hierarchy.roles'); $this->em = $em; $this->centerResolverDispatcher = $centerResolverDispatcher; + $this->logger = $logger; + $this->scopeResolverDispatcher = $scopeResolverDispatcher; } /** @@ -114,11 +125,32 @@ class AuthorizationHelper */ public function userHasAccess(User $user, $entity, $attribute) { - if (NULL === $center = $this->centerResolverDispatcher->resolveCenter($entity)) { - return false; - } + $center = $this->centerResolverDispatcher->resolveCenter($entity); + if (is_iterable($center)) { + foreach ($center as $c) { + if ($this->userHasAccessForCenter($user, $c, $entity, $attribute)) { + return true; + } + } + + return false; + } elseif ($center instanceof Center) { + return $this->userHasAccessForCenter($user, $center, $entity, $attribute); + } elseif (NULL === $center) { + return false; + } else { + throw new \UnexpectedValueException("could not resolver a center"); + } + } + + private function userHasAccessForCenter(User $user, Center $center, $entity, $attribute): bool + { if (!$this->userCanReachCenter($user, $center)) { + $this->logger->debug("user cannot reach center of entity", [ + 'center_name' => $center->getName(), + 'user' => $user->getUsername() + ]); return false; } @@ -132,24 +164,35 @@ class AuthorizationHelper if ($this->isRoleReached($attribute, $roleScope->getRole())) { //if yes, we have a right on something... // perform check on scope if necessary - if ($entity instanceof HasScopeInterface) { - $scope = $entity->getScope(); - if ($scope === NULL) { - return true; - } - if ($scope->getId() === $roleScope - ->getScope()->getId()) { - return true; - } + if ($this->scopeResolverDispatcher->isConcerned($entity)) { + $scope = $this->scopeResolverDispatcher->resolveScope($entity); + + if (NULL === $scope) { + return true; + } elseif (is_iterable($scope)) { + foreach ($scope as $s) { + if ($s === $roleScope->getScope()) { + return true; + } + } + } else { + if ($scope === $roleScope->getScope()) { + return true; + } + } } else { return true; } } } - } } + $this->logger->debug("user can reach center entity, but not role", [ + 'username' => $user->getUsername(), + 'center' => $center->getName() + ]); + return false; } diff --git a/src/Bundle/ChillMainBundle/Security/Resolver/CenterResolverDispatcher.php b/src/Bundle/ChillMainBundle/Security/Resolver/CenterResolverDispatcher.php index 6b1f2a897..1e2f9c02b 100644 --- a/src/Bundle/ChillMainBundle/Security/Resolver/CenterResolverDispatcher.php +++ b/src/Bundle/ChillMainBundle/Security/Resolver/CenterResolverDispatcher.php @@ -9,8 +9,6 @@ class CenterResolverDispatcher */ private iterable $resolvers = []; - private CenterResolverInterface $defaultResolver; - public function __construct(iterable $resolvers) { $this->resolvers = $resolvers; diff --git a/src/Bundle/ChillMainBundle/Security/Resolver/DefaultCenterResolver.php b/src/Bundle/ChillMainBundle/Security/Resolver/DefaultCenterResolver.php index 31531e0cc..e3e92164a 100644 --- a/src/Bundle/ChillMainBundle/Security/Resolver/DefaultCenterResolver.php +++ b/src/Bundle/ChillMainBundle/Security/Resolver/DefaultCenterResolver.php @@ -4,12 +4,13 @@ namespace Chill\MainBundle\Security\Resolver; use Chill\MainBundle\Entity\Center; use Chill\MainBundle\Entity\HasCenterInterface; +use Chill\MainBundle\Entity\HasCentersInterface; class DefaultCenterResolver implements CenterResolverInterface { public function supports($entity, ?array $options = []): bool { - return $entity instanceof HasCenterInterface; + return $entity instanceof HasCenterInterface || $entity instanceof HasCentersInterface; } /** @@ -20,7 +21,13 @@ class DefaultCenterResolver implements CenterResolverInterface */ public function resolveCenter($entity, ?array $options = []) { - return $entity->getCenter(); + if ($entity instanceof HasCenterInterface) { + return $entity->getCenter(); + } elseif ($entity instanceof HasCentersInterface) { + return $entity->getCenters(); + } else { + throw new \UnexpectedValueException("should be an instanceof"); + } } public static function getDefaultPriority(): int diff --git a/src/Bundle/ChillMainBundle/Security/Resolver/DefaultScopeResolver.php b/src/Bundle/ChillMainBundle/Security/Resolver/DefaultScopeResolver.php new file mode 100644 index 000000000..caf052764 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Security/Resolver/DefaultScopeResolver.php @@ -0,0 +1,42 @@ +getScope(); + } elseif ($entity instanceof HasScopesInterface) { + return $entity->getScopes(); + } else { + throw new \UnexpectedValueException("should be an instanceof %s or %s", + HasScopesInterface::class, HasScopeInterface::class); + } + } + + public function isConcerned($entity, ?array $options = []): bool + { + return $entity instanceof HasScopeInterface || $entity instanceof HasScopesInterface; + } + + public static function getDefaultPriority(): int + { + return -256; + } +} diff --git a/src/Bundle/ChillMainBundle/Security/Resolver/ScopeResolverDispatcher.php b/src/Bundle/ChillMainBundle/Security/Resolver/ScopeResolverDispatcher.php new file mode 100644 index 000000000..cc3f11560 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Security/Resolver/ScopeResolverDispatcher.php @@ -0,0 +1,44 @@ +resolvers = $resolvers; + } + + /** + * @param $entity + * @return Scope|Scope[]|iterable + */ + public function resolveScope($entity, ?array $options = []) + { + foreach ($this->resolvers as $resolver) { + if ($resolver->supports($entity, $options)) { + return $resolver->resolveScope($entity, $options); + } + } + + return null; + } + + public function isConcerned($entity, ?array $options = []): bool + { + foreach ($this->resolvers as $resolver) { + if ($resolver->supports($entity, $options)) { + return $resolver->isConcerned($entity, $options); + } + } + + return false; + } +} diff --git a/src/Bundle/ChillMainBundle/Security/Resolver/ScopeResolverInterface.php b/src/Bundle/ChillMainBundle/Security/Resolver/ScopeResolverInterface.php new file mode 100644 index 000000000..2f5669d0d --- /dev/null +++ b/src/Bundle/ChillMainBundle/Security/Resolver/ScopeResolverInterface.php @@ -0,0 +1,30 @@ +getAuthorizationHelper(); $entity = $this->getProphet()->prophesize(); - $entity->willImplement('\Chill\MainBundle\Entity\HasCenterInterface'); - $entity->willImplement('\Chill\MainBundle\Entity\HasScopeInterface'); + $entity->willImplement(HasCenterInterface::class); + $entity->willImplement(HasScopeInterface::class); $entity->getCenter()->willReturn($center); $entity->getScope()->willReturn($scopeA); $this->assertFalse($helper->userHasAccess($user, $entity->reveal(), 'CHILL_ROLE')); } + public function testUserHasAccess_MultiCenter_EntityWithoutScope() + { + $center = $this->prepareCenter(1, 'center'); + $centerB = $this->prepareCenter(1, 'centerB'); + $scopeB = $this->prepareScope(2, 'other'); //the user will be granted this scope + $user = $this->prepareUser(array( + array( + 'center' => $center, 'permissionsGroup' => array( + ['scope' => $scopeB, 'role' => 'CHILL_ROLE'] + ) + ) + )); + $helper = $this->getAuthorizationHelper(); + $entity = $this->getProphet()->prophesize(); + $entity->willImplement(HasCentersInterface::class); + $entity->getCenters()->willReturn([$center, $centerB]); + + $this->assertTrue($helper->userHasAccess($user, $entity->reveal(), 'CHILL_ROLE')); + } + + public function testUserHasNoAccess_MultiCenter_EntityWithoutScope() + { + $center = $this->prepareCenter(1, 'center'); + $centerB = $this->prepareCenter(1, 'centerB'); + $centerC = $this->prepareCenter(1, 'centerC'); + $scopeB = $this->prepareScope(2, 'other'); //the user will be granted this scope + $user = $this->prepareUser(array( + array( + 'center' => $center, 'permissionsGroup' => array( + ['scope' => $scopeB, 'role' => 'CHILL_ROLE'] + ) + ) + )); + $helper = $this->getAuthorizationHelper(); + $entity = $this->getProphet()->prophesize(); + $entity->willImplement(HasCentersInterface::class); + $entity->getCenters()->willReturn([$centerB, $centerC]); + + $this->assertFalse($helper->userHasAccess($user, $entity->reveal(), 'CHILL_ROLE')); + } + + public function testUserHasNoAccess_EntityMultiScope() + { + $centerA = $this->prepareCenter(1, 'center'); + $centerB = $this->prepareCenter(1, 'centerB'); + $scopeA = $this->prepareScope(2, 'other'); //the user will be granted this scope + $scopeB = $this->prepareScope(2, 'other'); //the user will be granted this scope + $scopeC = $this->prepareScope(2, 'other'); //the user will be granted this scope + $user = $this->prepareUser(array( + array( + 'center' => $centerA, 'permissionsGroup' => array( + ['scope' => $scopeA, 'role' => 'CHILL_ROLE'] + ) + ) + )); + $helper = $this->getAuthorizationHelper(); + $entity = $this->getProphet()->prophesize(); + $entity->willImplement(HasCentersInterface::class); + $entity->willImplement(HasScopesInterface::class); + $entity->getCenters()->willReturn([$centerA, $centerB]); + $entity->getScopes()->willReturn([$scopeB, $scopeC]); + + $this->assertFalse($helper->userHasAccess($user, $entity->reveal(), 'CHILL_ROLE')); + } + + public function testUserHasAccess_EntityMultiScope() + { + $centerA = $this->prepareCenter(1, 'center'); + $centerB = $this->prepareCenter(1, 'centerB'); + $scopeA = $this->prepareScope(2, 'other'); //the user will be granted this scope + $scopeB = $this->prepareScope(2, 'other'); //the user will be granted this scope + $user = $this->prepareUser(array( + array( + 'center' => $centerA, 'permissionsGroup' => array( + ['scope' => $scopeA, 'role' => 'CHILL_ROLE'] + ) + ) + )); + $helper = $this->getAuthorizationHelper(); + $entity = $this->getProphet()->prophesize(); + $entity->willImplement(HasCentersInterface::class); + $entity->willImplement(HasScopesInterface::class); + $entity->getCenters()->willReturn([$centerA, $centerB]); + $entity->getScopes()->willReturn([$scopeA, $scopeB]); + + $this->assertTrue($helper->userHasAccess($user, $entity->reveal(), 'CHILL_ROLE')); + } + + + /** * * @dataProvider dataProvider_getReachableCenters diff --git a/src/Bundle/ChillMainBundle/Tests/Security/Resolver/DefaultScopeResolverTest.php b/src/Bundle/ChillMainBundle/Tests/Security/Resolver/DefaultScopeResolverTest.php new file mode 100644 index 000000000..de6d19b5a --- /dev/null +++ b/src/Bundle/ChillMainBundle/Tests/Security/Resolver/DefaultScopeResolverTest.php @@ -0,0 +1,62 @@ +scopeResolver = new DefaultScopeResolver(); + } + + public function testHasScopeInterface() + { + $scope = new Scope(); + $entity = new class($scope) implements HasScopeInterface { + + public function __construct(Scope $scope) { + $this->scope = $scope; + } + + public function getScope() + { + return $this->scope; + } + + }; + + $this->assertTrue($this->scopeResolver->supports($entity)); + $this->assertTrue($this->scopeResolver->isConcerned($entity)); + $this->assertSame($scope, $this->scopeResolver->resolveScope($entity)); + } + + public function testHasScopesInterface() + { + $entity = new class($scopeA = new Scope(), $scopeB = new Scope()) implements HasScopesInterface { + + public function __construct(Scope $scopeA, Scope $scopeB) { + $this->scopes = [$scopeA, $scopeB]; + } + + public function getScopes(): iterable + { + return $this->scopes; + } + }; + + $this->assertTrue($this->scopeResolver->supports($entity)); + $this->assertTrue($this->scopeResolver->isConcerned($entity)); + $this->assertIsArray($this->scopeResolver->resolveScope($entity)); + $this->assertSame($scopeA, $this->scopeResolver->resolveScope($entity)[0]); + $this->assertSame($scopeB, $this->scopeResolver->resolveScope($entity)[1]); + } + +} diff --git a/src/Bundle/ChillMainBundle/Tests/Security/Resolver/ScopeResolverDispatcherTest.php b/src/Bundle/ChillMainBundle/Tests/Security/Resolver/ScopeResolverDispatcherTest.php new file mode 100644 index 000000000..2a36fd365 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Tests/Security/Resolver/ScopeResolverDispatcherTest.php @@ -0,0 +1,60 @@ +scopeResolverDispatcher = new ScopeResolverDispatcher([new DefaultScopeResolver()]); + } + + public function testHasScopeInterface() + { + $scope = new Scope(); + $entity = new class($scope) implements HasScopeInterface { + + public function __construct(Scope $scope) { + $this->scope = $scope; + } + + public function getScope() + { + return $this->scope; + } + + }; + + $this->assertTrue($this->scopeResolverDispatcher->isConcerned($entity)); + $this->assertSame($scope, $this->scopeResolverDispatcher->resolveScope($entity)); + } + + public function testHasScopesInterface() + { + $entity = new class($scopeA = new Scope(), $scopeB = new Scope()) implements HasScopesInterface { + + public function __construct(Scope $scopeA, Scope $scopeB) { + $this->scopes = [$scopeA, $scopeB]; + } + + public function getScopes(): iterable + { + return $this->scopes; + } + }; + + $this->assertTrue($this->scopeResolverDispatcher->isConcerned($entity)); + $this->assertIsArray($this->scopeResolverDispatcher->resolveScope($entity)); + $this->assertSame($scopeA, $this->scopeResolverDispatcher->resolveScope($entity)[0]); + $this->assertSame($scopeB, $this->scopeResolverDispatcher->resolveScope($entity)[1]); + } +} diff --git a/src/Bundle/ChillMainBundle/config/services/security.yaml b/src/Bundle/ChillMainBundle/config/services/security.yaml index c9703dbfe..0d820220b 100644 --- a/src/Bundle/ChillMainBundle/config/services/security.yaml +++ b/src/Bundle/ChillMainBundle/config/services/security.yaml @@ -8,11 +8,19 @@ services: arguments: - !tagged_iterator chill_main.center_resolver + Chill\MainBundle\Security\Resolver\ScopeResolverDispatcher: + arguments: + - !tagged_iterator chill_main.scope_resolver + # do not autowire the directory Security/Resolver Chill\MainBundle\Security\Resolver\DefaultCenterResolver: autoconfigure: true autowire: true + Chill\MainBundle\Security\Resolver\DefaultScopeResolver: + autoconfigure: true + autowire: true + # do not autowire the directory Security/Resolver Chill\MainBundle\Security\Resolver\ResolverTwigExtension: autoconfigure: true diff --git a/src/Bundle/ChillPersonBundle/Controller/AccompanyingPeriodController.php b/src/Bundle/ChillPersonBundle/Controller/AccompanyingPeriodController.php index 9cba2ec1d..255c6d9ba 100644 --- a/src/Bundle/ChillPersonBundle/Controller/AccompanyingPeriodController.php +++ b/src/Bundle/ChillPersonBundle/Controller/AccompanyingPeriodController.php @@ -23,7 +23,7 @@ namespace Chill\PersonBundle\Controller; use Chill\PersonBundle\Privacy\PrivacyEvent; -use Chill\PersonBundle\Repository\AccompanyingPeriodACLAwareRepository; +use Chill\PersonBundle\Repository\AccompanyingPeriodACLAwareRepositoryInterface; use Chill\PersonBundle\Security\Authorization\AccompanyingPeriodVoter; use Doctrine\DBAL\Exception; use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter; @@ -56,10 +56,10 @@ class AccompanyingPeriodController extends AbstractController */ protected $validator; - protected AccompanyingPeriodACLAwareRepository $accompanyingPeriodACLAwareRepository; + protected AccompanyingPeriodACLAwareRepositoryInterface $accompanyingPeriodACLAwareRepository; public function __construct( - AccompanyingPeriodACLAwareRepository $accompanyingPeriodACLAwareRepository, + AccompanyingPeriodACLAwareRepositoryInterface $accompanyingPeriodACLAwareRepository, EventDispatcherInterface $eventDispatcher, ValidatorInterface $validator ) { diff --git a/src/Bundle/ChillPersonBundle/Entity/AccompanyingPeriod.php b/src/Bundle/ChillPersonBundle/Entity/AccompanyingPeriod.php index 1c375d051..5672b7f09 100644 --- a/src/Bundle/ChillPersonBundle/Entity/AccompanyingPeriod.php +++ b/src/Bundle/ChillPersonBundle/Entity/AccompanyingPeriod.php @@ -24,6 +24,8 @@ namespace Chill\PersonBundle\Entity; use Chill\MainBundle\Doctrine\Model\TrackUpdateInterface; use Chill\MainBundle\Doctrine\Model\TrackCreationInterface; +use Chill\MainBundle\Entity\HasCentersInterface; +use Chill\MainBundle\Entity\HasScopesInterface; use Chill\MainBundle\Entity\Scope; use Chill\MainBundle\Entity\Address; use Chill\PersonBundle\Entity\AccompanyingPeriod\AccompanyingPeriodWork; @@ -52,7 +54,8 @@ use Symfony\Component\Validator\Constraints as Assert; * "accompanying_period"=AccompanyingPeriod::class * }) */ -class AccompanyingPeriod implements TrackCreationInterface, TrackUpdateInterface +class AccompanyingPeriod implements TrackCreationInterface, TrackUpdateInterface, + HasScopesInterface, HasCentersInterface { /** * Mark an accompanying period as "occasional" @@ -809,11 +812,16 @@ class AccompanyingPeriod implements TrackCreationInterface, TrackUpdateInterface return $this; } + /** + * @return iterable|Collection + */ public function getScopes(): Collection { return $this->scopes; } + + public function addScope(Scope $scope): self { $this->scopes[] = $scope; @@ -1040,4 +1048,20 @@ class AccompanyingPeriod implements TrackCreationInterface, TrackUpdateInterface return 'none'; } } + + public function getCenters(): ?iterable + {dump(__METHOD__); + foreach ($this->getPersons() as $person) { + if (!in_array($person->getCenter(), $centers ?? []) + && NULL !== $person->getCenter()) { + $centers[] = $person->getCenter(); + } + } + + dump($centers); + + return $centers ?? null; + } + + } diff --git a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php index cdc439e32..c8cca82f5 100644 --- a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php +++ b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php @@ -4,10 +4,11 @@ namespace Chill\PersonBundle\Repository; use Chill\MainBundle\Security\Authorization\AuthorizationHelper; use Chill\MainBundle\Security\Resolver\CenterResolverDispatcher; +use Chill\PersonBundle\Entity\AccompanyingPeriod; use Chill\PersonBundle\Entity\Person; use Symfony\Component\Security\Core\Security; -final class AccompanyingPeriodACLAwareRepository +final class AccompanyingPeriodACLAwareRepository implements AccompanyingPeriodACLAwareRepositoryInterface { private AccompanyingPeriodRepository $accompanyingPeriodRepository; private Security $security; @@ -29,7 +30,6 @@ final class AccompanyingPeriodACLAwareRepository int $limit = null, int $offset = null ): array { - dump(__METHOD__); $qb = $this->accompanyingPeriodRepository->createQueryBuilder('ap'); $scopes = $this->authorizationHelper ->getReachableCircles($this->security->getUser(), $role, @@ -42,12 +42,30 @@ final class AccompanyingPeriodACLAwareRepository $qb ->join('ap.participations', 'participation') ->where($qb->expr()->eq('participation.person', ':person')) + ->andWhere( + $qb->expr()->orX( + 'ap.confidential = FALSE', + $qb->expr()->eq('ap.user', ':user') + ) + ) + ->andWhere( + $qb->expr()->orX( + $qb->expr()->neq('ap.step', ':draft'), + $qb->expr()->eq('ap.createdBy', ':creator') + ) + ) + ->setParameter('draft', AccompanyingPeriod::STEP_DRAFT) ->setParameter('person', $person) - ; + ->setParameter('user', $this->security->getUser()) + ->setParameter('creator', $this->security->getUser()) + ; // add join condition for scopes - $orx = $qb->expr()->orX(); + $orx = $qb->expr()->orX( + $qb->expr()->eq('ap.step', ':draft') + ); + foreach ($scopes as $key => $scope) { - $orx->add($qb->expr()->in('ap.scopes', ':scope_'.$key)); + $orx->add($qb->expr()->isMemberOf(':scope_'.$key, 'ap.scopes')); $qb->setParameter('scope_'.$key, $scope); } $qb->andWhere($orx); diff --git a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepositoryInterface.php b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepositoryInterface.php new file mode 100644 index 000000000..a5798d3e8 --- /dev/null +++ b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepositoryInterface.php @@ -0,0 +1,16 @@ +