From 15ff92257c3d758dc93efc120ea1d2996b38b32a Mon Sep 17 00:00:00 2001 From: Tchama Date: Thu, 17 Jan 2019 16:21:35 +0100 Subject: [PATCH 1/5] wip.. adapt EventVoter to sf3 --- Menu/PersonMenuBuilder.php | 24 +--- Resources/config/services/authorization.yml | 2 + Resources/config/services/menu.yml | 3 +- Resources/views/Event/listByPerson.html.twig | 4 +- Security/Authorization/EventVoter.php | 117 ++++++++++++++----- 5 files changed, 103 insertions(+), 47 deletions(-) diff --git a/Menu/PersonMenuBuilder.php b/Menu/PersonMenuBuilder.php index c7470fe2a..130622c2c 100644 --- a/Menu/PersonMenuBuilder.php +++ b/Menu/PersonMenuBuilder.php @@ -2,21 +2,15 @@ namespace Chill\EventBundle\Menu; -use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; +use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; use Symfony\Component\Translation\TranslatorInterface; use Symfony\Component\Security\Core\Role\Role; use Knp\Menu\MenuItem; use Chill\MainBundle\Routing\LocalMenuBuilderInterface; -use Chill\MainBundle\Security\Authorization\AuthorizationHelper; use Chill\EventBundle\Security\Authorization\EventVoter; class PersonMenuBuilder implements LocalMenuBuilderInterface { - /** - * - * @var TokenStorageInterface - */ - protected $tokenStorage; /** * @@ -26,18 +20,16 @@ class PersonMenuBuilder implements LocalMenuBuilderInterface /** * - * @var AuthorizationHelper + * @var AuthorizationCheckerInterface */ - protected $authorizationHelper; + protected $authorizationChecker; public function __construct( - AuthorizationHelper $authorizationHelper, - TokenStorageInterface $tokenStorage, + AuthorizationCheckerInterface $authorizationChecker, TranslatorInterface $translator ) { - $this->tokenStorage = $tokenStorage; + $this->authorizationChecker = $authorizationChecker; $this->translator = $translator; - $this->authorizationHelper = $authorizationHelper; } @@ -46,11 +38,7 @@ class PersonMenuBuilder implements LocalMenuBuilderInterface /* @var $person \Chill\PersonBundle\Entity\Person */ $person = $parameters['person']; - $user = $this->tokenStorage->getToken()->getUser(); - $roleSee = new Role(EventVoter::SEE); - - // ASK use authorizationHelper or authorizationChecker ?? - if ($this->authorizationHelper->userHasAccess($user, $person, $roleSee)) { + if ($this->authorizationChecker->isGranted(EventVoter::SEE, $person)) { $menu->addChild($this->translator->trans('Events participation'), [ 'route' => 'chill_event__list_by_person', diff --git a/Resources/config/services/authorization.yml b/Resources/config/services/authorization.yml index d44f1e9d9..131469f65 100644 --- a/Resources/config/services/authorization.yml +++ b/Resources/config/services/authorization.yml @@ -2,7 +2,9 @@ services: chill_event.event_voter: class: Chill\EventBundle\Security\Authorization\EventVoter arguments: + - "@security.access.decision_manager" - "@chill.main.security.authorization.helper" + - "@logger" tags: - { name: chill.role } - { name: security.voter } diff --git a/Resources/config/services/menu.yml b/Resources/config/services/menu.yml index 209d208a7..a2451122f 100644 --- a/Resources/config/services/menu.yml +++ b/Resources/config/services/menu.yml @@ -1,8 +1,7 @@ services: Chill\EventBundle\Menu\PersonMenuBuilder: arguments: - $authorizationHelper: '@Chill\MainBundle\Security\Authorization\AuthorizationHelper' - $tokenStorage: '@Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface' + $authorizationChecker: '@Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface' $translator: '@Symfony\Component\Translation\TranslatorInterface' tags: - { name: 'chill.menu_builder' } \ No newline at end of file diff --git a/Resources/views/Event/listByPerson.html.twig b/Resources/views/Event/listByPerson.html.twig index 9c174bb92..cc99cff4f 100644 --- a/Resources/views/Event/listByPerson.html.twig +++ b/Resources/views/Event/listByPerson.html.twig @@ -50,7 +50,8 @@ diff --git a/Security/Authorization/EventVoter.php b/Security/Authorization/EventVoter.php index 8f74e649a..77548f1a5 100644 --- a/Security/Authorization/EventVoter.php +++ b/Security/Authorization/EventVoter.php @@ -9,6 +9,10 @@ use Chill\MainBundle\Security\ProvideRoleHierarchyInterface; use Chill\EventBundle\Entity\Event; use Chill\MainBundle\Security\Authorization\AuthorizationHelper; use Chill\MainBundle\Entity\User; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; +use Psr\Log\LoggerInterface; + /** * Description of EventVoter @@ -18,53 +22,114 @@ use Chill\MainBundle\Entity\User; */ class EventVoter extends AbstractChillVoter implements ProvideRoleHierarchyInterface { - const SEE = 'CHILL_EVENT_SEE'; const SEE_DETAILS = 'CHILL_EVENT_SEE_DETAILS'; const CREATE = 'CHILL_EVENT_CREATE'; const UPDATE = 'CHILL_EVENT_UPDATE'; + const ROLES = [ + self::SEE, + self::SEE_DETAILS, + self::CREATE, + self::UPDATE + ]; + + /** + * @var AuthorizationHelper + */ protected $authorizationHelper; - public function __construct(AuthorizationHelper $helper) + /** + * @var AccessDecisionManagerInterface + */ + protected $accessDecisionManager; + + /** + * @var LoggerInterface + */ + protected $logger; + + public function __construct( + AccessDecisionManagerInterface $accessDecisionManager, + AuthorizationHelper $authorizationHelper, + LoggerInterface $logger + ) { - $this->authorizationHelper = $helper; + $this->accessDecisionManager = $accessDecisionManager; + $this->authorizationHelper = $authorizationHelper; + $this->logger = $logger; } - protected function getSupportedAttributes() + public function supports($attribute, $subject) { - return array(self::SEE, self::SEE_DETAILS, - self::CREATE, self::UPDATE); + return ($subject instanceof Event && in_array($attribute, self::ROLES)) + || + ($subject instanceof Person && \in_array($attribute, [ self::CREATE, self::SEE ])) + || + (NULL === $subject && $attribute === self::SEE ) + ; } - - protected function getSupportedClasses() + + /** + * + * @param string $attribute + * @param Event $subject + * @param TokenInterface $token + * @return boolean + */ + protected function voteOnAttribute($attribute, $subject, TokenInterface $token) { - return array(Event::class); - } - - protected function isGranted($attribute, $event, $user = null) - { - if (!$user instanceof User) { - return false; - } + $this->logger->debug(sprintf("Voting from %s class", self::class)); - return $this->authorizationHelper->userHasAccess($user, $event, $attribute); + if (!$token->getUser() instanceof User) { + return false; + } + + if ($subject instanceof Event) { + if ($subject->getPerson() === null) { + throw new \LogicException("You should associate a person with event " + . "in order to check autorizations"); + } + $person = $subject->getPerson(); + + } elseif ($subject instanceof Person) { + $person = $subject; + + } 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( + $token->getUser(), + $subject, + $attribute + ); } - + + public function getRoles() { - return $this->getSupportedAttributes(); + return self::ROLES; + } + + public function getRolesWithHierarchy() + { + return [ + 'Event' => self::ROLES + ]; } public function getRolesWithoutScope() { - return null; + return []; } - - public function getRolesWithHierarchy() - { - return [ 'Event' => $this->getRoles() ]; - } - } From a10d48642fa4ad8908ba4bc7e58be5248dfc5f09 Mon Sep 17 00:00:00 2001 From: Tchama Date: Wed, 23 Jan 2019 09:11:53 +0100 Subject: [PATCH 2/5] wip.. corrections --- Resources/views/Event/listByPerson.html.twig | 2 +- Security/Authorization/EventVoter.php | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Resources/views/Event/listByPerson.html.twig b/Resources/views/Event/listByPerson.html.twig index cc99cff4f..f0337b300 100644 --- a/Resources/views/Event/listByPerson.html.twig +++ b/Resources/views/Event/listByPerson.html.twig @@ -50,7 +50,6 @@ diff --git a/Security/Authorization/EventVoter.php b/Security/Authorization/EventVoter.php index 77548f1a5..f3ef499f0 100644 --- a/Security/Authorization/EventVoter.php +++ b/Security/Authorization/EventVoter.php @@ -9,6 +9,7 @@ use Chill\MainBundle\Security\ProvideRoleHierarchyInterface; use Chill\EventBundle\Entity\Event; use Chill\MainBundle\Security\Authorization\AuthorizationHelper; use Chill\MainBundle\Entity\User; +use Chill\PersonBundle\Security\Authorization\PersonVoter; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Psr\Log\LoggerInterface; @@ -86,11 +87,11 @@ class EventVoter extends AbstractChillVoter implements ProvideRoleHierarchyInter } if ($subject instanceof Event) { - if ($subject->getPerson() === null) { + if ($subject->getId() === null) { throw new \LogicException("You should associate a person with event " . "in order to check autorizations"); } - $person = $subject->getPerson(); + $person = $subject->getId(); // liaison event --> person } elseif ($subject instanceof Person) { $person = $subject; From 2d295d8e433164ae11e36e00155cf6ef3f80859b Mon Sep 17 00:00:00 2001 From: Tchama Date: Fri, 25 Jan 2019 13:45:34 +0100 Subject: [PATCH 3/5] test --- Security/Authorization/EventVoter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Security/Authorization/EventVoter.php b/Security/Authorization/EventVoter.php index f3ef499f0..c41a200b1 100644 --- a/Security/Authorization/EventVoter.php +++ b/Security/Authorization/EventVoter.php @@ -91,7 +91,7 @@ class EventVoter extends AbstractChillVoter implements ProvideRoleHierarchyInter throw new \LogicException("You should associate a person with event " . "in order to check autorizations"); } - $person = $subject->getId(); // liaison event --> person + $person = $subject->getId(); // liaison event --> person } elseif ($subject instanceof Person) { $person = $subject; From 31876ee8b6200027e1456ae7639de52695f55346 Mon Sep 17 00:00:00 2001 From: Tchama Date: Fri, 25 Jan 2019 15:39:44 +0100 Subject: [PATCH 4/5] wip.. EVENT_SEE and _CREATE works in twig test, not in php test --- Menu/PersonMenuBuilder.php | 13 +++++++++++-- Resources/views/Event/listByPerson.html.twig | 7 +++++++ Security/Authorization/EventVoter.php | 15 ++++++--------- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/Menu/PersonMenuBuilder.php b/Menu/PersonMenuBuilder.php index 130622c2c..7411d482b 100644 --- a/Menu/PersonMenuBuilder.php +++ b/Menu/PersonMenuBuilder.php @@ -4,7 +4,6 @@ namespace Chill\EventBundle\Menu; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; use Symfony\Component\Translation\TranslatorInterface; -use Symfony\Component\Security\Core\Role\Role; use Knp\Menu\MenuItem; use Chill\MainBundle\Routing\LocalMenuBuilderInterface; use Chill\EventBundle\Security\Authorization\EventVoter; @@ -36,7 +35,7 @@ class PersonMenuBuilder implements LocalMenuBuilderInterface public function buildMenu($menuId, MenuItem $menu, array $parameters) { /* @var $person \Chill\PersonBundle\Entity\Person */ - $person = $parameters['person']; + $person = $parameters['person'] ?? null; if ($this->authorizationChecker->isGranted(EventVoter::SEE, $person)) { @@ -49,7 +48,17 @@ class PersonMenuBuilder implements LocalMenuBuilderInterface ->setExtras([ 'order' => 500 ]); + } + //// + else { + dump('not see'); } + if ($this->authorizationChecker->isGranted(EventVoter::CREATE, $person)) { + dump('create'); + } else { + dump('not create'); + } + //// } public static function getMenuIds(): array diff --git a/Resources/views/Event/listByPerson.html.twig b/Resources/views/Event/listByPerson.html.twig index f0337b300..82742674f 100644 --- a/Resources/views/Event/listByPerson.html.twig +++ b/Resources/views/Event/listByPerson.html.twig @@ -50,6 +50,13 @@