From 7923b5a1ef25f7209fedde12d955e4e61af2005f Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Fri, 14 Jun 2024 15:35:50 +0200 Subject: [PATCH 01/18] initial commit --- .changes/unreleased/Feature-20240614-153537.yaml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changes/unreleased/Feature-20240614-153537.yaml diff --git a/.changes/unreleased/Feature-20240614-153537.yaml b/.changes/unreleased/Feature-20240614-153537.yaml new file mode 100644 index 000000000..c16d49b2d --- /dev/null +++ b/.changes/unreleased/Feature-20240614-153537.yaml @@ -0,0 +1,7 @@ +kind: Feature +body: The behavoir of the voters for stored objects is adjusted so as to limit edit + and delete possibilities to users related to the activity, social action or workflow + entity. +time: 2024-06-14T15:35:37.582159301+02:00 +custom: + Issue: "286" From 65c41e6fa957b9616d9e9be3f5feec9080f23d21 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Fri, 14 Jun 2024 16:48:09 +0200 Subject: [PATCH 02/18] Add StoredObjectVoterInterface An interface is defined that can be implemented by each context-specific voter in the future. --- .../StoredObjectVoterInterface.php | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php new file mode 100644 index 000000000..47b93eabb --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php @@ -0,0 +1,22 @@ + Date: Fri, 14 Jun 2024 17:22:27 +0200 Subject: [PATCH 03/18] Refactorize StoredObjectVoter.php The StoredObjectVoter.php has been refactorized to handle context-specific voters.\ This way we can check if the context-specific voter should handle the authorization or not.\ If not, there is a simple fallback to check on the USER_ROLE. --- .../Authorization/StoredObjectVoter.php | 28 ++++++++++++++----- .../config/services/voter.yaml | 14 ++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) create mode 100644 src/Bundle/ChillDocStoreBundle/config/services/voter.yaml diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php index 2e253cf3c..ecfc56615 100644 --- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php @@ -15,6 +15,7 @@ use Chill\DocStoreBundle\Entity\StoredObject; use Chill\DocStoreBundle\Security\Guard\DavTokenAuthenticationEventSubscriber; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\Voter\Voter; +use Symfony\Component\Security\Core\Security; /** * Voter for the content of a stored object. @@ -23,6 +24,14 @@ use Symfony\Component\Security\Core\Authorization\Voter\Voter; */ class StoredObjectVoter extends Voter { + private $security; + private $storedObjectVoters; + + public function __construct(Security $security, iterable $storedObjectVoters) { + $this->security = $security; + $this->storedObjectVoters = $storedObjectVoters; + } + protected function supports($attribute, $subject): bool { return StoredObjectRoleEnum::tryFrom($attribute) instanceof StoredObjectRoleEnum @@ -43,13 +52,18 @@ class StoredObjectVoter extends Voter return false; } - $askedRole = StoredObjectRoleEnum::from($attribute); - $tokenRoleAuthorization = - $token->getAttribute(DavTokenAuthenticationEventSubscriber::ACTIONS); + // Loop through context-specific voters + foreach ($this->storedObjectVoters as $storedObjectVoter) { + if ($storedObjectVoter->supports($attribute, $subject)) { + return $storedObjectVoter->voteOnAttribute($attribute, $subject, $token); + } + } - return match ($askedRole) { - StoredObjectRoleEnum::SEE => StoredObjectRoleEnum::EDIT === $tokenRoleAuthorization || StoredObjectRoleEnum::SEE === $tokenRoleAuthorization, - StoredObjectRoleEnum::EDIT => StoredObjectRoleEnum::EDIT === $tokenRoleAuthorization - }; + // User role-based fallback + if ($this->security->isGranted('ROLE_USER')) { + return true; + } + + return false; } } diff --git a/src/Bundle/ChillDocStoreBundle/config/services/voter.yaml b/src/Bundle/ChillDocStoreBundle/config/services/voter.yaml new file mode 100644 index 000000000..922d29cba --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/config/services/voter.yaml @@ -0,0 +1,14 @@ +services: + _defaults: + autowire: true + autoconfigure: true + Chill\DocStoreBundle\Security\Authorization\StoredObjectVoter: + arguments: + $storedObjectVoters: + # context specific voters + - '@accompanying_course_document_voter' + tags: + - { name: security.voter } + + accompanying_course_document_voter: + class: Chill\DocStoreBundle\Security\Authorization\AccompanyingCourseDocumentVoter From e9a9262fae52274da1012d070182f4825175def9 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Fri, 14 Jun 2024 17:27:22 +0200 Subject: [PATCH 04/18] Add config voter.yaml The voter.yaml was not configured to be taken into account. Now added\ to ChillDocStoreExtension.php --- .../DependencyInjection/ChillDocStoreExtension.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php b/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php index fe9aeecfa..5f87199a3 100644 --- a/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php +++ b/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php @@ -42,6 +42,7 @@ class ChillDocStoreExtension extends Extension implements PrependExtensionInterf $loader->load('services/fixtures.yaml'); $loader->load('services/form.yaml'); $loader->load('services/templating.yaml'); + $loader->load('services/voter.yaml'); } public function prepend(ContainerBuilder $container) From 4b82e67952983c7b2942e693596b467a4e75833c Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 19 Jun 2024 09:51:21 +0200 Subject: [PATCH 05/18] Type-hint $subject in StoredObjectVoterInterface.php Since the subject passed to these voters should\ always be of the type StoredObject, type-hinting\ added. --- .../Security/Authorization/StoredObjectVoterInterface.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php index 47b93eabb..a3a76e79a 100644 --- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php @@ -11,12 +11,13 @@ declare(strict_types=1); namespace ChillDocStoreBundle\Security\Authorization; +use Chill\DocStoreBundle\Entity\StoredObject; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; interface StoredObjectVoterInterface { - public function supports(string $attribute, $subject): bool; + public function supports(string $attribute, StoredObject $subject): bool; - public function voteOnAttribute(string $attribute, $subject, TokenInterface $token): bool; + public function voteOnAttribute(string $attribute, StoredObject $subject, TokenInterface $token): bool; } From ad4fe8024092e7b365ee0f6aba7a5987ca964d18 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 19 Jun 2024 09:52:59 +0200 Subject: [PATCH 06/18] Add fall-back right for ROLE_ADMIN Within the StoredObjectVoter.php also the admin\ user should be able to edit documents as a fall-back --- .../Security/Authorization/StoredObjectVoter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php index ecfc56615..781c2c542 100644 --- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php @@ -60,7 +60,7 @@ class StoredObjectVoter extends Voter } // User role-based fallback - if ($this->security->isGranted('ROLE_USER')) { + if ($this->security->isGranted('ROLE_USER') || $this->security->isGranted('ROLE_ADMIN')) { return true; } From 04a48f22ad85abd8fdb13155c805ec709a7757c9 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 19 Jun 2024 10:00:10 +0200 Subject: [PATCH 07/18] Use constructor property promotion In accordance with php8.1 use property promotion\ within the constructor method. Less clutter. --- .../Security/Authorization/StoredObjectVoter.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php index 781c2c542..7e2becab4 100644 --- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php @@ -24,12 +24,8 @@ use Symfony\Component\Security\Core\Security; */ class StoredObjectVoter extends Voter { - private $security; - private $storedObjectVoters; - public function __construct(Security $security, iterable $storedObjectVoters) { - $this->security = $security; - $this->storedObjectVoters = $storedObjectVoters; + public function __construct(private readonly Security $security, private readonly iterable $storedObjectVoters) { } protected function supports($attribute, $subject): bool From e015f71bb001ca00662e9f97a82cc8095efdb284 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 19 Jun 2024 10:02:25 +0200 Subject: [PATCH 08/18] Rename voter.yaml file to security.yaml For consistency with other bundles voters are\ registered under the security.yaml file. --- .../DependencyInjection/ChillDocStoreExtension.php | 2 +- .../config/services/{voter.yaml => security.yaml} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/Bundle/ChillDocStoreBundle/config/services/{voter.yaml => security.yaml} (100%) diff --git a/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php b/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php index 5f87199a3..a0fcc2d5d 100644 --- a/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php +++ b/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php @@ -42,7 +42,7 @@ class ChillDocStoreExtension extends Extension implements PrependExtensionInterf $loader->load('services/fixtures.yaml'); $loader->load('services/form.yaml'); $loader->load('services/templating.yaml'); - $loader->load('services/voter.yaml'); + $loader->load('services/security.yaml'); } public function prepend(ContainerBuilder $container) diff --git a/src/Bundle/ChillDocStoreBundle/config/services/voter.yaml b/src/Bundle/ChillDocStoreBundle/config/services/security.yaml similarity index 100% rename from src/Bundle/ChillDocStoreBundle/config/services/voter.yaml rename to src/Bundle/ChillDocStoreBundle/config/services/security.yaml From e0828b1f0fa9d9c255a6a1189492e33d7c2ca31d Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 19 Jun 2024 10:16:39 +0200 Subject: [PATCH 09/18] Use service tags to inject all voters into StoredObjectVoter.php Instead of manually injecting services into StoredObjectVoter\ config is added to automatically tag each service that implements\ StoredObjectVoterInterface.php --- .../DependencyInjection/ChillDocStoreExtension.php | 3 +++ .../ChillDocStoreBundle/config/services/security.yaml | 9 ++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php b/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php index a0fcc2d5d..9f277e716 100644 --- a/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php +++ b/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php @@ -14,6 +14,7 @@ namespace Chill\DocStoreBundle\DependencyInjection; use Chill\DocStoreBundle\Controller\StoredObjectApiController; use Chill\DocStoreBundle\Security\Authorization\AccompanyingCourseDocumentVoter; use Chill\DocStoreBundle\Security\Authorization\PersonDocumentVoter; +use ChillDocStoreBundle\Security\Authorization\StoredObjectVoterInterface; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface; @@ -35,6 +36,8 @@ class ChillDocStoreExtension extends Extension implements PrependExtensionInterf $container->setParameter('chill_doc_store', $config); + $container->registerForAutoconfiguration(StoredObjectVoterInterface::class)->addTag('stored_object_voter'); + $loader = new Loader\YamlFileLoader($container, new FileLocator(__DIR__.'/../config')); $loader->load('services.yaml'); $loader->load('services/controller.yaml'); diff --git a/src/Bundle/ChillDocStoreBundle/config/services/security.yaml b/src/Bundle/ChillDocStoreBundle/config/services/security.yaml index 922d29cba..c57eb63c5 100644 --- a/src/Bundle/ChillDocStoreBundle/config/services/security.yaml +++ b/src/Bundle/ChillDocStoreBundle/config/services/security.yaml @@ -4,11 +4,10 @@ services: autoconfigure: true Chill\DocStoreBundle\Security\Authorization\StoredObjectVoter: arguments: - $storedObjectVoters: - # context specific voters - - '@accompanying_course_document_voter' + $storedObjectVoters: !tagged_iterator stored_object_voter tags: - { name: security.voter } - accompanying_course_document_voter: - class: Chill\DocStoreBundle\Security\Authorization\AccompanyingCourseDocumentVoter + Chill\DocStoreBundle\Security\Authorization\AccompanyingCourseDocumentVoter: + tags: + - { name: security.voter } From 482f279dc5635a79f0ee14c21670e570c9a5ba07 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 19 Jun 2024 10:21:24 +0200 Subject: [PATCH 10/18] Implement StoredObjectVoterInterface An interface was created to be implemented by Stored Doc voters\ these will automatically be tagged and injected into DocStoreVoter. --- .../Authorization/AccompanyingCourseDocumentVoter.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php index 5febc7e42..93243b1f4 100644 --- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php @@ -19,11 +19,12 @@ use Chill\MainBundle\Security\Authorization\VoterHelperInterface; use Chill\MainBundle\Security\ProvideRoleHierarchyInterface; use Chill\PersonBundle\Entity\AccompanyingPeriod; use Chill\PersonBundle\Security\Authorization\AccompanyingPeriodVoter; +use ChillDocStoreBundle\Security\Authorization\StoredObjectVoterInterface; use Psr\Log\LoggerInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Security; -class AccompanyingCourseDocumentVoter extends AbstractChillVoter implements ProvideRoleHierarchyInterface +class AccompanyingCourseDocumentVoter extends AbstractChillVoter implements ProvideRoleHierarchyInterface, StoredObjectVoterInterface { final public const CREATE = 'CHILL_ACCOMPANYING_COURSE_DOCUMENT_CREATE'; @@ -70,12 +71,12 @@ class AccompanyingCourseDocumentVoter extends AbstractChillVoter implements Prov return []; } - protected function supports($attribute, $subject): bool + public function supports($attribute, $subject): bool { return $this->voterHelper->supports($attribute, $subject); } - protected function voteOnAttribute($attribute, $subject, TokenInterface $token): bool + public function voteOnAttribute($attribute, $subject, TokenInterface $token): bool { if (!$token->getUser() instanceof User) { return false; From 427f232ab805125a010a9a4a78cad62e795a32be Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Thu, 20 Jun 2024 10:53:33 +0200 Subject: [PATCH 11/18] Correct namespace and use statement for StoredObjectVoterInterface.php The namespace was formed wrong and needed adjustment --- .../Security/Authorization/AccompanyingCourseDocumentVoter.php | 1 - .../Security/Authorization/StoredObjectVoterInterface.php | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php index 93243b1f4..dac08b05b 100644 --- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php @@ -19,7 +19,6 @@ use Chill\MainBundle\Security\Authorization\VoterHelperInterface; use Chill\MainBundle\Security\ProvideRoleHierarchyInterface; use Chill\PersonBundle\Entity\AccompanyingPeriod; use Chill\PersonBundle\Security\Authorization\AccompanyingPeriodVoter; -use ChillDocStoreBundle\Security\Authorization\StoredObjectVoterInterface; use Psr\Log\LoggerInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Security; diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php index a3a76e79a..c0fd7e09f 100644 --- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php @@ -9,7 +9,7 @@ declare(strict_types=1); * the LICENSE file that was distributed with this source code. */ -namespace ChillDocStoreBundle\Security\Authorization; +namespace Chill\DocStoreBundle\Security\Authorization; use Chill\DocStoreBundle\Entity\StoredObject; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; From d26fa6bde6e8113cdb2e6d4aded57635a1d8254a Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Thu, 20 Jun 2024 15:17:56 +0200 Subject: [PATCH 12/18] Implement voting logic: separation of concerns A separate AccompanyingCourseDocumentStoredObjectVoter was\ created to handle the specific access to a Stored object\ related to an Accompanying Course Document. --- .../AccompanyingCourseDocumentRepository.php | 13 +++++ ...panyingCourseDocumentStoredObjectVoter.php | 50 +++++++++++++++++++ ...nyingPeriodWorkEvaluationDocumentVoter.php | 7 +-- 3 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentStoredObjectVoter.php diff --git a/src/Bundle/ChillDocStoreBundle/Repository/AccompanyingCourseDocumentRepository.php b/src/Bundle/ChillDocStoreBundle/Repository/AccompanyingCourseDocumentRepository.php index 2679993c4..239b88b90 100644 --- a/src/Bundle/ChillDocStoreBundle/Repository/AccompanyingCourseDocumentRepository.php +++ b/src/Bundle/ChillDocStoreBundle/Repository/AccompanyingCourseDocumentRepository.php @@ -12,6 +12,7 @@ declare(strict_types=1); namespace Chill\DocStoreBundle\Repository; use Chill\DocStoreBundle\Entity\AccompanyingCourseDocument; +use Chill\DocStoreBundle\Entity\StoredObject; use Chill\PersonBundle\Entity\AccompanyingPeriod; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityRepository; @@ -45,6 +46,17 @@ class AccompanyingCourseDocumentRepository implements ObjectRepository return $qb->getQuery()->getSingleScalarResult(); } + public function findLinkedCourseDocument(int $storedObjectId): ?AccompanyingCourseDocument { + + $qb = $this->repository->createQueryBuilder('d'); + $query = $qb->leftJoin('d.storedObject', 'do') + ->where('do.id = :storedObjectId') + ->setParameter('storedObjectId', $storedObjectId) + ->getQuery(); + + return $query->getResult(); + } + public function find($id): ?AccompanyingCourseDocument { return $this->repository->find($id); @@ -69,4 +81,5 @@ class AccompanyingCourseDocumentRepository implements ObjectRepository { return AccompanyingCourseDocument::class; } + } diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentStoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentStoredObjectVoter.php new file mode 100644 index 000000000..db1499a99 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentStoredObjectVoter.php @@ -0,0 +1,50 @@ +repository->findLinkedCourseDocument($subject->getId())); + } + + public function voteOnAttribute(string $attribute, StoredObject $subject, TokenInterface $token): bool + { + if (!$token->getUser() instanceof User) { + return false; + } + + // Retrieve the related accompanying course document + $accompanyingCourseDocument = $this->repository->findLinkedCourseDocument($subject->getId()); + + // Determine the attribute to pass to AccompanyingCourseDocumentVoter + $voterAttribute = ($attribute === self::SEE_AND_EDIT) ? AccompanyingCourseDocumentVoter::UPDATE : AccompanyingCourseDocumentVoter::SEE_DETAILS; + + // Check access using AccompanyingCourseDocumentVoter + if ($this->accompanyingCourseDocumentVoter->voteOnAttribute($voterAttribute, $accompanyingCourseDocument, $token)) { + // TODO implement logic to check for associated workflow + return true; + } else { + return false; + } + } +} diff --git a/src/Bundle/ChillPersonBundle/Security/Authorization/AccompanyingPeriodWorkEvaluationDocumentVoter.php b/src/Bundle/ChillPersonBundle/Security/Authorization/AccompanyingPeriodWorkEvaluationDocumentVoter.php index 77c131cd9..8f4a1b995 100644 --- a/src/Bundle/ChillPersonBundle/Security/Authorization/AccompanyingPeriodWorkEvaluationDocumentVoter.php +++ b/src/Bundle/ChillPersonBundle/Security/Authorization/AccompanyingPeriodWorkEvaluationDocumentVoter.php @@ -11,6 +11,7 @@ declare(strict_types=1); namespace Chill\PersonBundle\Security\Authorization; +use Chill\DocStoreBundle\Security\Authorization\StoredObjectVoterInterface; use Chill\PersonBundle\Entity\AccompanyingPeriod\AccompanyingPeriodWorkEvaluationDocument; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; @@ -21,13 +22,13 @@ use Symfony\Component\Security\Core\Authorization\Voter\Voter; * * Delegates to the sames authorization than for Evalution */ -class AccompanyingPeriodWorkEvaluationDocumentVoter extends Voter +class AccompanyingPeriodWorkEvaluationDocumentVoter extends Voter implements StoredObjectVoterInterface { final public const SEE = 'CHILL_MAIN_ACCOMPANYING_PERIOD_WORK_EVALUATION_DOCUMENT_SHOW'; public function __construct(private readonly AccessDecisionManagerInterface $accessDecisionManager) {} - protected function supports($attribute, $subject) + public function supports($attribute, $subject): bool { return $subject instanceof AccompanyingPeriodWorkEvaluationDocument && self::SEE === $attribute; @@ -39,7 +40,7 @@ class AccompanyingPeriodWorkEvaluationDocumentVoter extends Voter * * @return bool|void */ - protected function voteOnAttribute($attribute, $subject, TokenInterface $token): bool + public function voteOnAttribute($attribute, $subject, TokenInterface $token): bool { return match ($attribute) { self::SEE => $this->accessDecisionManager->decide( From 760d65b9723d66b4bd3dd1aec55cfd784864a962 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Thu, 20 Jun 2024 17:27:21 +0200 Subject: [PATCH 13/18] Remove implementation of StoredObjectVoterInterface in AccompanyingCourseDocumentVoter.php This implementation has been moved to the voter\ AccompanyingCourseDocumentStoredObjectVoter.php --- .../Security/Authorization/AccompanyingCourseDocumentVoter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php index dac08b05b..7a46184cb 100644 --- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentVoter.php @@ -23,7 +23,7 @@ use Psr\Log\LoggerInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Security; -class AccompanyingCourseDocumentVoter extends AbstractChillVoter implements ProvideRoleHierarchyInterface, StoredObjectVoterInterface +class AccompanyingCourseDocumentVoter extends AbstractChillVoter implements ProvideRoleHierarchyInterface { final public const CREATE = 'CHILL_ACCOMPANYING_COURSE_DOCUMENT_CREATE'; From 3d40db749386c47e4e6b690b41a53113f18fce29 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Thu, 20 Jun 2024 17:28:19 +0200 Subject: [PATCH 14/18] Refactor AccompanyingCourseDocumentRepository.php Build where clause using StoredObject directly instead\ of based on it's id. --- .../Repository/AccompanyingCourseDocumentRepository.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Bundle/ChillDocStoreBundle/Repository/AccompanyingCourseDocumentRepository.php b/src/Bundle/ChillDocStoreBundle/Repository/AccompanyingCourseDocumentRepository.php index 239b88b90..93cf00ecb 100644 --- a/src/Bundle/ChillDocStoreBundle/Repository/AccompanyingCourseDocumentRepository.php +++ b/src/Bundle/ChillDocStoreBundle/Repository/AccompanyingCourseDocumentRepository.php @@ -46,12 +46,11 @@ class AccompanyingCourseDocumentRepository implements ObjectRepository return $qb->getQuery()->getSingleScalarResult(); } - public function findLinkedCourseDocument(int $storedObjectId): ?AccompanyingCourseDocument { + public function findLinkedCourseDocument(StoredObject $storedObject): ?AccompanyingCourseDocument { $qb = $this->repository->createQueryBuilder('d'); - $query = $qb->leftJoin('d.storedObject', 'do') - ->where('do.id = :storedObjectId') - ->setParameter('storedObjectId', $storedObjectId) + $query = $qb->where('d.storedObject = :storedObject') + ->setParameter('storedObject', $storedObject) ->getQuery(); return $query->getResult(); From 73797b98f670c20535b8f1ca03a5f7de5862d224 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Thu, 20 Jun 2024 17:32:09 +0200 Subject: [PATCH 15/18] Add WorkflowDocumentService and use in StoredObject voters A WorkflowDocumentService was created that can be injected\ in context-specific StoredObject voters that need to check whether\ the document in question is attached to a workflow. --- ...panyingCourseDocumentStoredObjectVoter.php | 31 ++++++++++------ .../Service/WorkflowDocumentService.php | 35 +++++++++++++++++++ 2 files changed, 56 insertions(+), 10 deletions(-) create mode 100644 src/Bundle/ChillDocStoreBundle/Service/WorkflowDocumentService.php diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentStoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentStoredObjectVoter.php index db1499a99..931378a61 100644 --- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentStoredObjectVoter.php +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentStoredObjectVoter.php @@ -2,13 +2,15 @@ namespace ChillDocStoreBundle\Security\Authorization; +use Chill\DocStoreBundle\Entity\AccompanyingCourseDocument; use Chill\DocStoreBundle\Entity\StoredObject; use Chill\DocStoreBundle\Repository\AccompanyingCourseDocumentRepository; use Chill\DocStoreBundle\Security\Authorization\AccompanyingCourseDocumentVoter; use Chill\DocStoreBundle\Security\Authorization\StoredObjectVoterInterface; +use Chill\DocStoreBundle\Service\WorkflowDocumentService; use Chill\MainBundle\Entity\User; +use Chill\MainBundle\Entity\Workflow\EntityWorkflow; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; -use Symfony\Component\Security\Core\Security; class AccompanyingCourseDocumentStoredObjectVoter implements StoredObjectVoterInterface { @@ -16,15 +18,15 @@ class AccompanyingCourseDocumentStoredObjectVoter implements StoredObjectVoterIn public function __construct( private readonly AccompanyingCourseDocumentRepository $repository, - private readonly Security $security, - private readonly AccompanyingCourseDocumentVoter $accompanyingCourseDocumentVoter + private readonly AccompanyingCourseDocumentVoter $accompanyingCourseDocumentVoter, + private readonly WorkflowDocumentService $workflowDocumentService ){ } public function supports(string $attribute, StoredObject $subject): bool { // check if the stored object is linked to an AccompanyingCourseDocument - return !empty($this->repository->findLinkedCourseDocument($subject->getId())); + return $this->repository->findLinkedCourseDocument($subject) instanceof AccompanyingCourseDocument; } public function voteOnAttribute(string $attribute, StoredObject $subject, TokenInterface $token): bool @@ -34,17 +36,26 @@ class AccompanyingCourseDocumentStoredObjectVoter implements StoredObjectVoterIn } // Retrieve the related accompanying course document - $accompanyingCourseDocument = $this->repository->findLinkedCourseDocument($subject->getId()); + $accompanyingCourseDocument = $this->repository->findLinkedCourseDocument($subject); // Determine the attribute to pass to AccompanyingCourseDocumentVoter - $voterAttribute = ($attribute === self::SEE_AND_EDIT) ? AccompanyingCourseDocumentVoter::UPDATE : AccompanyingCourseDocumentVoter::SEE_DETAILS; + $voterAttribute = match($attribute) { + self::SEE_AND_EDIT => AccompanyingCourseDocumentVoter::UPDATE, + default => AccompanyingCourseDocumentVoter::SEE_DETAILS, + }; // Check access using AccompanyingCourseDocumentVoter - if ($this->accompanyingCourseDocumentVoter->voteOnAttribute($voterAttribute, $accompanyingCourseDocument, $token)) { - // TODO implement logic to check for associated workflow - return true; - } else { + if (false === $this->accompanyingCourseDocumentVoter->voteOnAttribute($voterAttribute, $accompanyingCourseDocument, $token)) { return false; } + + // Check if entity is related to a workflow, if so, check if user can apply transition + $relatedWorkflow = $this->workflowDocumentService->getRelatedWorkflow($accompanyingCourseDocument); + + if ($relatedWorkflow instanceof EntityWorkflow){ + return $this->workflowDocumentService->canApplyTransition($relatedWorkflow); + } + + return true; } } diff --git a/src/Bundle/ChillDocStoreBundle/Service/WorkflowDocumentService.php b/src/Bundle/ChillDocStoreBundle/Service/WorkflowDocumentService.php new file mode 100644 index 000000000..498348b96 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Service/WorkflowDocumentService.php @@ -0,0 +1,35 @@ +repository->findByRelatedEntity(get_class($entity), $entity->getId()); + } + + public function canApplyTransition(EntityWorkflow $entityWorkflow): bool + { + if ($entityWorkflow->isFinal()) { + return false; + } + + $currentUser = $this->security->getUser(); + if ($entityWorkflow->getCurrentStep()->getAllDestUser()->contains($currentUser)) { + return true; + } + + return false; + } + +} From 1310d53589c4fbc0ca5ce6080aee18bcf195cb15 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 26 Jun 2024 13:45:15 +0200 Subject: [PATCH 16/18] Implement context-specific voters for all current entities that can be linked to a document For reusability an AbstractStoredObjectVoter was created and a StoredObjectVoterInterface. A WorkflowDocumentService checks whether the StoredObject is involved in a workflow. --- .../Repository/ActivityRepository.php | 17 ++++- .../AccompanyingCourseDocumentRepository.php | 14 ++-- ...ssociatedEntityToStoredObjectInterface.php | 10 +++ .../Repository/PersonDocumentRepository.php | 13 +++- ...panyingCourseDocumentStoredObjectVoter.php | 61 ----------------- .../Authorization/StoredObjectVoter.php | 8 ++- .../StoredObjectVoterInterface.php | 5 +- .../AbstractStoredObjectVoter.php | 67 +++++++++++++++++++ .../AccompanyingCourseStoredObjectVoter.php | 46 +++++++++++++ ...gPeriodWorkEvaluationStoredObjectVoter.php | 50 ++++++++++++++ .../ActivityStoredObjectVoter.php | 50 ++++++++++++++ .../EventStoredObjectVoter.php | 49 ++++++++++++++ .../PersonStoredObjectVoter.php | 49 ++++++++++++++ .../Service/WorkflowDocumentService.php | 14 ++-- src/Bundle/ChillEventBundle/Entity/Event.php | 2 +- .../Repository/EventRepository.php | 59 ++++++++++++++-- .../Workflow/EntityWorkflowRepository.php | 16 +++++ ...PeriodWorkEvaluationDocumentRepository.php | 14 +++- 18 files changed, 456 insertions(+), 88 deletions(-) create mode 100644 src/Bundle/ChillDocStoreBundle/Repository/AssociatedEntityToStoredObjectInterface.php delete mode 100644 src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentStoredObjectVoter.php create mode 100644 src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/AbstractStoredObjectVoter.php create mode 100644 src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/AccompanyingCourseStoredObjectVoter.php create mode 100644 src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/AccompanyingPeriodWorkEvaluationStoredObjectVoter.php create mode 100644 src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/ActivityStoredObjectVoter.php create mode 100644 src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/EventStoredObjectVoter.php create mode 100644 src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/PersonStoredObjectVoter.php diff --git a/src/Bundle/ChillActivityBundle/Repository/ActivityRepository.php b/src/Bundle/ChillActivityBundle/Repository/ActivityRepository.php index a7025d4a9..54672c6b7 100644 --- a/src/Bundle/ChillActivityBundle/Repository/ActivityRepository.php +++ b/src/Bundle/ChillActivityBundle/Repository/ActivityRepository.php @@ -12,6 +12,8 @@ declare(strict_types=1); namespace Chill\ActivityBundle\Repository; use Chill\ActivityBundle\Entity\Activity; +use Chill\DocStoreBundle\Entity\StoredObject; +use Chill\DocStoreBundle\Repository\AssociatedEntityToStoredObjectInterface; use Chill\PersonBundle\Entity\AccompanyingPeriod; use Chill\PersonBundle\Entity\Person; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; @@ -23,7 +25,7 @@ use Doctrine\Persistence\ManagerRegistry; * @method Activity[] findAll() * @method Activity[] findBy(array $criteria, array $orderBy = null, $limit = null, $offset = null) */ -class ActivityRepository extends ServiceEntityRepository +class ActivityRepository extends ServiceEntityRepository implements AssociatedEntityToStoredObjectInterface { public function __construct(ManagerRegistry $registry) { @@ -97,4 +99,17 @@ class ActivityRepository extends ServiceEntityRepository return $qb->getQuery()->getResult(); } + + public function findAssociatedEntityToStoredObject(StoredObject $storedObject): ?object + { + $qb = $this->createQueryBuilder('a'); + $query = $qb + ->join('a.documents', 'ad') + ->join('ad.storedObject', 'so') + ->where('so.id = :storedObjectId') + ->setParameter('storedObjectId', $storedObject->getId()) + ->getQuery(); + + return $query->getResult(); + } } diff --git a/src/Bundle/ChillDocStoreBundle/Repository/AccompanyingCourseDocumentRepository.php b/src/Bundle/ChillDocStoreBundle/Repository/AccompanyingCourseDocumentRepository.php index 93cf00ecb..246c7f2d9 100644 --- a/src/Bundle/ChillDocStoreBundle/Repository/AccompanyingCourseDocumentRepository.php +++ b/src/Bundle/ChillDocStoreBundle/Repository/AccompanyingCourseDocumentRepository.php @@ -19,7 +19,7 @@ use Doctrine\ORM\EntityRepository; use Doctrine\ORM\QueryBuilder; use Doctrine\Persistence\ObjectRepository; -class AccompanyingCourseDocumentRepository implements ObjectRepository +class AccompanyingCourseDocumentRepository implements ObjectRepository, AssociatedEntityToStoredObjectInterface { private readonly EntityRepository $repository; @@ -46,12 +46,12 @@ class AccompanyingCourseDocumentRepository implements ObjectRepository return $qb->getQuery()->getSingleScalarResult(); } - public function findLinkedCourseDocument(StoredObject $storedObject): ?AccompanyingCourseDocument { - + public function findAssociatedEntityToStoredObject(StoredObject $storedObject): ?object + { $qb = $this->repository->createQueryBuilder('d'); $query = $qb->where('d.storedObject = :storedObject') - ->setParameter('storedObject', $storedObject) - ->getQuery(); + ->setParameter('storedObject', $storedObject) + ->getQuery(); return $query->getResult(); } @@ -66,7 +66,7 @@ class AccompanyingCourseDocumentRepository implements ObjectRepository return $this->repository->findAll(); } - public function findBy(array $criteria, ?array $orderBy = null, $limit = null, $offset = null) + public function findBy(array $criteria, ?array $orderBy = null, $limit = null, $offset = null): array { return $this->repository->findBy($criteria, $orderBy, $limit, $offset); } @@ -76,7 +76,7 @@ class AccompanyingCourseDocumentRepository implements ObjectRepository return $this->findOneBy($criteria); } - public function getClassName() + public function getClassName(): string { return AccompanyingCourseDocument::class; } diff --git a/src/Bundle/ChillDocStoreBundle/Repository/AssociatedEntityToStoredObjectInterface.php b/src/Bundle/ChillDocStoreBundle/Repository/AssociatedEntityToStoredObjectInterface.php new file mode 100644 index 000000000..5349251c5 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Repository/AssociatedEntityToStoredObjectInterface.php @@ -0,0 +1,10 @@ + */ -readonly class PersonDocumentRepository implements ObjectRepository +readonly class PersonDocumentRepository implements ObjectRepository, AssociatedEntityToStoredObjectInterface { private EntityRepository $repository; @@ -53,4 +54,14 @@ readonly class PersonDocumentRepository implements ObjectRepository { return PersonDocument::class; } + + public function findAssociatedEntityToStoredObject(StoredObject $storedObject): ?object + { + $qb = $this->repository->createQueryBuilder('d'); + $query = $qb->where('d.storedObject = :storedObject') + ->setParameter('storedObject', $storedObject) + ->getQuery(); + + return $query->getResult(); + } } diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentStoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentStoredObjectVoter.php deleted file mode 100644 index 931378a61..000000000 --- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/AccompanyingCourseDocumentStoredObjectVoter.php +++ /dev/null @@ -1,61 +0,0 @@ -repository->findLinkedCourseDocument($subject) instanceof AccompanyingCourseDocument; - } - - public function voteOnAttribute(string $attribute, StoredObject $subject, TokenInterface $token): bool - { - if (!$token->getUser() instanceof User) { - return false; - } - - // Retrieve the related accompanying course document - $accompanyingCourseDocument = $this->repository->findLinkedCourseDocument($subject); - - // Determine the attribute to pass to AccompanyingCourseDocumentVoter - $voterAttribute = match($attribute) { - self::SEE_AND_EDIT => AccompanyingCourseDocumentVoter::UPDATE, - default => AccompanyingCourseDocumentVoter::SEE_DETAILS, - }; - - // Check access using AccompanyingCourseDocumentVoter - if (false === $this->accompanyingCourseDocumentVoter->voteOnAttribute($voterAttribute, $accompanyingCourseDocument, $token)) { - return false; - } - - // Check if entity is related to a workflow, if so, check if user can apply transition - $relatedWorkflow = $this->workflowDocumentService->getRelatedWorkflow($accompanyingCourseDocument); - - if ($relatedWorkflow instanceof EntityWorkflow){ - return $this->workflowDocumentService->canApplyTransition($relatedWorkflow); - } - - return true; - } -} diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php index 7e2becab4..3bb5fa396 100644 --- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoter.php @@ -48,15 +48,19 @@ class StoredObjectVoter extends Voter return false; } + $attributeAsEnum = StoredObjectRoleEnum::from($attribute); + // Loop through context-specific voters foreach ($this->storedObjectVoters as $storedObjectVoter) { - if ($storedObjectVoter->supports($attribute, $subject)) { - return $storedObjectVoter->voteOnAttribute($attribute, $subject, $token); + if ($storedObjectVoter->supports($attributeAsEnum, $subject)) { + return $storedObjectVoter->voteOnAttribute($attributeAsEnum, $subject, $token); } } // User role-based fallback if ($this->security->isGranted('ROLE_USER') || $this->security->isGranted('ROLE_ADMIN')) { + // TODO: this maybe considered as a security issue, as all authenticated users can reach a stored object which + // is potentially detached from an existing entity. return true; } diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php index c0fd7e09f..516722654 100644 --- a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoterInterface.php @@ -12,12 +12,13 @@ declare(strict_types=1); namespace Chill\DocStoreBundle\Security\Authorization; use Chill\DocStoreBundle\Entity\StoredObject; +use Chill\DocStoreBundle\Security\Authorization\StoredObjectRoleEnum; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; interface StoredObjectVoterInterface { - public function supports(string $attribute, StoredObject $subject): bool; + public function supports(StoredObjectRoleEnum $attribute, StoredObject $subject): bool; - public function voteOnAttribute(string $attribute, StoredObject $subject, TokenInterface $token): bool; + public function voteOnAttribute(StoredObjectRoleEnum $attribute, StoredObject $subject, TokenInterface $token): bool; } diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/AbstractStoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/AbstractStoredObjectVoter.php new file mode 100644 index 000000000..7ca73f206 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/AbstractStoredObjectVoter.php @@ -0,0 +1,67 @@ +getClass(); + return $this->getRepository()->findAssociatedEntityToStoredObject($subject) instanceof $class; + } + + public function voteOnAttribute(StoredObjectRoleEnum $attribute, StoredObject $subject, TokenInterface $token): bool + { + if (!$token->getUser() instanceof User) { + return false; + } + + // Retrieve the related accompanying course document + $entity = $this->getRepository()->findAssociatedEntityToStoredObject($subject); + + // Determine the attribute to pass to AccompanyingCourseDocumentVoter + $voterAttribute = $this->attributeToRole($attribute); + + if (false === $this->security->isGranted($voterAttribute, $entity)) { + return false; + } + + if ($this->canBeAssociatedWithWorkflow()) { + if (null === $this->workflowDocumentService) { + throw new \LogicException("Provide a workflow document service"); + } + + return $this->workflowDocumentService->notBlockedByWorkflow($entity); + } + + return true; + } +} diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/AccompanyingCourseStoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/AccompanyingCourseStoredObjectVoter.php new file mode 100644 index 000000000..a41ea3067 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/AccompanyingCourseStoredObjectVoter.php @@ -0,0 +1,46 @@ +repository; + } + + protected function attributeToRole(StoredObjectRoleEnum $attribute): string + { + return match ($attribute) { + StoredObjectRoleEnum::EDIT => AccompanyingCourseDocumentVoter::UPDATE, + StoredObjectRoleEnum::SEE => AccompanyingCourseDocumentVoter::SEE_DETAILS, + }; + } + + protected function getClass(): string + { + return AccompanyingCourseDocument::class; + } + + protected function canBeAssociatedWithWorkflow(): bool + { + return true; + } +} diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/AccompanyingPeriodWorkEvaluationStoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/AccompanyingPeriodWorkEvaluationStoredObjectVoter.php new file mode 100644 index 000000000..423730767 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/AccompanyingPeriodWorkEvaluationStoredObjectVoter.php @@ -0,0 +1,50 @@ +repository; + } + + /** + * @inheritDoc + */ + protected function getClass(): string + { + return AccompanyingPeriodWorkEvaluationDocument::class; + } + + protected function attributeToRole(StoredObjectRoleEnum $attribute): string + { + //Question: there is no update/edit check in AccompanyingPeriodWorkEvaluationDocumentVoter, so for both SEE and EDIT of the + // stored object I check with SEE right in AccompanyingPeriodWorkEvaluationDocumentVoter, correct? + return match ($attribute) { + StoredObjectRoleEnum::SEE, StoredObjectRoleEnum::EDIT => AccompanyingPeriodWorkEvaluationDocumentVoter::SEE, + }; + } + + protected function canBeAssociatedWithWorkflow(): bool + { + return true; + } +} diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/ActivityStoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/ActivityStoredObjectVoter.php new file mode 100644 index 000000000..a36535005 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/ActivityStoredObjectVoter.php @@ -0,0 +1,50 @@ +repository; + } + + /** + * @inheritDoc + */ + protected function getClass(): string + { + return Activity::class; + } + + protected function attributeToRole(StoredObjectRoleEnum $attribute): string + { + return match ($attribute) { + StoredObjectRoleEnum::EDIT => ActivityVoter::UPDATE, + StoredObjectRoleEnum::SEE => ActivityVoter::SEE_DETAILS, + }; + } + + protected function canBeAssociatedWithWorkflow(): bool + { + return false; + } +} diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/EventStoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/EventStoredObjectVoter.php new file mode 100644 index 000000000..218ce1980 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/EventStoredObjectVoter.php @@ -0,0 +1,49 @@ +repository; + } + + /** + * @inheritDoc + */ + protected function getClass(): string + { + return Event::class; + } + + protected function attributeToRole(StoredObjectRoleEnum $attribute): string + { + return match ($attribute) { + StoredObjectRoleEnum::EDIT => EventVoter::UPDATE, + StoredObjectRoleEnum::SEE => EventVoter::SEE_DETAILS, + }; + } + + protected function canBeAssociatedWithWorkflow(): bool + { + return false; + } +} diff --git a/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/PersonStoredObjectVoter.php b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/PersonStoredObjectVoter.php new file mode 100644 index 000000000..6c3b0b807 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Security/Authorization/StoredObjectVoters/PersonStoredObjectVoter.php @@ -0,0 +1,49 @@ +repository; + } + + /** + * @inheritDoc + */ + protected function getClass(): string + { + return PersonDocument::class; + } + + protected function attributeToRole(StoredObjectRoleEnum $attribute): string + { + return match ($attribute) { + StoredObjectRoleEnum::EDIT => PersonDocumentVoter::UPDATE, + StoredObjectRoleEnum::SEE => PersonDocumentVoter::SEE_DETAILS, + }; + } + + protected function canBeAssociatedWithWorkflow(): bool + { + return true; + } +} diff --git a/src/Bundle/ChillDocStoreBundle/Service/WorkflowDocumentService.php b/src/Bundle/ChillDocStoreBundle/Service/WorkflowDocumentService.php index 498348b96..f796fe56e 100644 --- a/src/Bundle/ChillDocStoreBundle/Service/WorkflowDocumentService.php +++ b/src/Bundle/ChillDocStoreBundle/Service/WorkflowDocumentService.php @@ -13,19 +13,19 @@ class WorkflowDocumentService { } - public function getRelatedWorkflow($entity): ?EntityWorkflow + public function notBlockedByWorkflow($entity): bool { - return $this->repository->findByRelatedEntity(get_class($entity), $entity->getId()); - } + /** + * @var EntityWorkflow + */ + $workflow = $this->repository->findByRelatedEntity(get_class($entity), $entity->getId()); - public function canApplyTransition(EntityWorkflow $entityWorkflow): bool - { - if ($entityWorkflow->isFinal()) { + if ($workflow->isFinal()) { return false; } $currentUser = $this->security->getUser(); - if ($entityWorkflow->getCurrentStep()->getAllDestUser()->contains($currentUser)) { + if ($workflow->getCurrentStep()->getAllDestUser()->contains($currentUser)) { return true; } diff --git a/src/Bundle/ChillEventBundle/Entity/Event.php b/src/Bundle/ChillEventBundle/Entity/Event.php index 6f7ee7ef0..be5969363 100644 --- a/src/Bundle/ChillEventBundle/Entity/Event.php +++ b/src/Bundle/ChillEventBundle/Entity/Event.php @@ -31,7 +31,7 @@ use Symfony\Component\Validator\Constraints as Assert; /** * Class Event. */ -#[ORM\Entity(repositoryClass: \Chill\EventBundle\Repository\EventRepository::class)] +#[ORM\Entity] #[ORM\HasLifecycleCallbacks] #[ORM\Table(name: 'chill_event_event')] class Event implements HasCenterInterface, HasScopeInterface, TrackCreationInterface, TrackUpdateInterface diff --git a/src/Bundle/ChillEventBundle/Repository/EventRepository.php b/src/Bundle/ChillEventBundle/Repository/EventRepository.php index 7406748b4..24eb095ec 100644 --- a/src/Bundle/ChillEventBundle/Repository/EventRepository.php +++ b/src/Bundle/ChillEventBundle/Repository/EventRepository.php @@ -11,17 +11,66 @@ declare(strict_types=1); namespace Chill\EventBundle\Repository; +use Chill\DocStoreBundle\Entity\StoredObject; +use Chill\DocStoreBundle\Repository\AssociatedEntityToStoredObjectInterface; use Chill\EventBundle\Entity\Event; -use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; -use Doctrine\Persistence\ManagerRegistry; +use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\EntityRepository; +use Doctrine\ORM\QueryBuilder; +use Doctrine\Persistence\ObjectRepository; /** * Class EventRepository. */ -class EventRepository extends ServiceEntityRepository +class EventRepository implements ObjectRepository, AssociatedEntityToStoredObjectInterface { - public function __construct(ManagerRegistry $registry) + private readonly EntityRepository $repository; + + public function __construct(EntityManagerInterface $entityManager) { - parent::__construct($registry, Event::class); + $this->repository = $entityManager->getRepository(Event::class); + } + + public function createQueryBuilder(string $alias, ?string $indexBy = null): QueryBuilder + { + return $this->repository->createQueryBuilder($alias, $indexBy); + } + + public function findAssociatedEntityToStoredObject(StoredObject $storedObject): ?object + { + $qb = $this->createQueryBuilder('e'); + $query = $qb + ->join('e.documents', 'ed') + ->join('ed.storedObject', 'so') + ->where('so.id = :storedObjectId') + ->setParameter('storedObjectId', $storedObject->getId()) + ->getQuery(); + + return $query->getResult(); + } + + public function find($id) + { + return $this->repository->find($id); + } + + public function findAll(): array + { + return $this->repository->findAll(); + } + + public function findBy(array $criteria, ?array $orderBy = null, ?int $limit = null, ?int $offset = null): array + { + return $this->repository->findBy($criteria, $orderBy, $limit, $offset); + } + + public function findOneBy(array $criteria) + { + return $this->repository->findOneBy($criteria); + } + + public function getClassName(): string + { + return Event::class; } } diff --git a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php index 66b3ab379..7d2e047d3 100644 --- a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php +++ b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php @@ -99,6 +99,22 @@ class EntityWorkflowRepository implements ObjectRepository return $this->repository->findAll(); } + public function findByRelatedEntity($entityClass, $relatedEntityId): ?EntityWorkflow + { + $qb = $this->repository->createQueryBuilder('w'); + + $query = $qb->where( + $qb->expr()->andX( + $qb->expr()->eq('w.relatedEntityClass', ':entity_class'), + $qb->expr()->eq('w.relatedEntityId', ':entity_id'), + ) + )->setParameter('entity_class', $entityClass) + ->setParameter('entity_id', $relatedEntityId); + + return $query->getQuery()->getResult(); + + } + /** * @param mixed|null $limit * @param mixed|null $offset diff --git a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriod/AccompanyingPeriodWorkEvaluationDocumentRepository.php b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriod/AccompanyingPeriodWorkEvaluationDocumentRepository.php index 59bb3f915..60dcdf1b1 100644 --- a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriod/AccompanyingPeriodWorkEvaluationDocumentRepository.php +++ b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriod/AccompanyingPeriodWorkEvaluationDocumentRepository.php @@ -11,12 +11,14 @@ declare(strict_types=1); namespace Chill\PersonBundle\Repository\AccompanyingPeriod; +use Chill\DocStoreBundle\Entity\StoredObject; +use Chill\DocStoreBundle\Repository\AssociatedEntityToStoredObjectInterface; use Chill\PersonBundle\Entity\AccompanyingPeriod\AccompanyingPeriodWorkEvaluationDocument; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityRepository; use Doctrine\Persistence\ObjectRepository; -class AccompanyingPeriodWorkEvaluationDocumentRepository implements ObjectRepository +class AccompanyingPeriodWorkEvaluationDocumentRepository implements ObjectRepository, AssociatedEntityToStoredObjectInterface { private readonly EntityRepository $repository; @@ -58,4 +60,14 @@ class AccompanyingPeriodWorkEvaluationDocumentRepository implements ObjectReposi { return AccompanyingPeriodWorkEvaluationDocument::class; } + + public function findAssociatedEntityToStoredObject(StoredObject $storedObject): ?object + { + $qb = $this->repository->createQueryBuilder('ed'); + $query = $qb->where('ed.storedObject = :storedObject') + ->setParameter('storedObject', $storedObject) + ->getQuery(); + + return $query->getResult(); + } } From bd36735cb16be8e7cd640aaa598383fa4d562a56 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 26 Jun 2024 14:06:02 +0200 Subject: [PATCH 17/18] Ensure single result when retrieving activity and event linked to stored object Although a many-to-many relationship exists between these entities and stored object, only one activity or event will ever be linked to a single stored object. For extra safety measure we return a single result in the repository to ensure our voters will keep working. --- .../ChillActivityBundle/Repository/ActivityRepository.php | 8 +++++++- .../ChillEventBundle/Repository/EventRepository.php | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Bundle/ChillActivityBundle/Repository/ActivityRepository.php b/src/Bundle/ChillActivityBundle/Repository/ActivityRepository.php index 54672c6b7..4d621d56e 100644 --- a/src/Bundle/ChillActivityBundle/Repository/ActivityRepository.php +++ b/src/Bundle/ChillActivityBundle/Repository/ActivityRepository.php @@ -17,6 +17,8 @@ use Chill\DocStoreBundle\Repository\AssociatedEntityToStoredObjectInterface; use Chill\PersonBundle\Entity\AccompanyingPeriod; use Chill\PersonBundle\Entity\Person; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; +use Doctrine\ORM\NonUniqueResultException; +use Doctrine\ORM\NoResultException; use Doctrine\Persistence\ManagerRegistry; /** @@ -100,6 +102,10 @@ class ActivityRepository extends ServiceEntityRepository implements AssociatedEn return $qb->getQuery()->getResult(); } + /** + * @throws NonUniqueResultException + * @throws NoResultException + */ public function findAssociatedEntityToStoredObject(StoredObject $storedObject): ?object { $qb = $this->createQueryBuilder('a'); @@ -110,6 +116,6 @@ class ActivityRepository extends ServiceEntityRepository implements AssociatedEn ->setParameter('storedObjectId', $storedObject->getId()) ->getQuery(); - return $query->getResult(); + return $query->getSingleResult(); } } diff --git a/src/Bundle/ChillEventBundle/Repository/EventRepository.php b/src/Bundle/ChillEventBundle/Repository/EventRepository.php index 24eb095ec..b720866f2 100644 --- a/src/Bundle/ChillEventBundle/Repository/EventRepository.php +++ b/src/Bundle/ChillEventBundle/Repository/EventRepository.php @@ -16,6 +16,8 @@ use Chill\DocStoreBundle\Repository\AssociatedEntityToStoredObjectInterface; use Chill\EventBundle\Entity\Event; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityRepository; +use Doctrine\ORM\NonUniqueResultException; +use Doctrine\ORM\NoResultException; use Doctrine\ORM\QueryBuilder; use Doctrine\Persistence\ObjectRepository; @@ -36,6 +38,10 @@ class EventRepository implements ObjectRepository, AssociatedEntityToStoredObjec return $this->repository->createQueryBuilder($alias, $indexBy); } + /** + * @throws NonUniqueResultException + * @throws NoResultException + */ public function findAssociatedEntityToStoredObject(StoredObject $storedObject): ?object { $qb = $this->createQueryBuilder('e'); @@ -46,7 +52,7 @@ class EventRepository implements ObjectRepository, AssociatedEntityToStoredObjec ->setParameter('storedObjectId', $storedObject->getId()) ->getQuery(); - return $query->getResult(); + return $query->getSingleResult(); } public function find($id) From d3956319caf28f98e5512f568a4ad138aa7dd223 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 26 Jun 2024 14:56:25 +0200 Subject: [PATCH 18/18] Add test for AccompayingCourseStoredObjectVoter Mainly to check the voteOnAttribute method, by mocking a scenario where a person is allowed to see/edit an AccompanyingCourseDocument and not. --- ...ccompanyingCourseStoredObjectVoterTest.php | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 src/Bundle/ChillDocStoreBundle/Tests/Security/Authorization/AccompanyingCourseStoredObjectVoterTest.php diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Security/Authorization/AccompanyingCourseStoredObjectVoterTest.php b/src/Bundle/ChillDocStoreBundle/Tests/Security/Authorization/AccompanyingCourseStoredObjectVoterTest.php new file mode 100644 index 000000000..f7ad25987 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Tests/Security/Authorization/AccompanyingCourseStoredObjectVoterTest.php @@ -0,0 +1,106 @@ +repository = $this->createMock(AccompanyingCourseDocumentRepository::class); + $this->security = $this->createMock(Security::class); + $this->workflowDocumentService = $this->createMock(WorkflowDocumentService::class); + + $this->voter = new AccompanyingCourseStoredObjectVoter( + $this->repository, + $this->security, + $this->workflowDocumentService + ); + } + + private function setupMockObjects(): array + { + $user = $this->createMock(User::class); + $token = $this->createMock(TokenInterface::class); + $subject = $this->createMock(StoredObject::class); + $entity = $this->createMock(AccompanyingCourseDocument::class); + + return [$user, $token, $subject, $entity]; + } + + private function setupMocksForVoteOnAttribute(User $user, TokenInterface $token, bool $isGrantedForAccCourseDocument, AccompanyingCourseDocument $entity, bool $workflowAllowed): void + { + // Set up token to return user + $token->method('getUser')->willReturn($user); + + // Mock the return of an AccompanyingCourseDocument by the repository + $this->repository->method('findAssociatedEntityToStoredObject')->willReturn($entity); + + // Mock attributeToRole to return appropriate role + $this->voter->method('attributeToRole')->willReturn(AccompanyingCourseDocumentVoter::SEE_DETAILS); + + // Mock scenario where user is allowed to see_details of the AccompanyingCourseDocument + $this->security->method('isGranted')->willReturnMap([ + [[AccompanyingCourseDocumentVoter::SEE_DETAILS, $entity], $isGrantedForAccCourseDocument], + ]); + + // Mock case where user is blocked or not by workflow + $this->workflowDocumentService->method('notBlockedByWorkflow')->willReturn($workflowAllowed); + } + + public function testVoteOnAttributeAllowed(): void + { + list($user, $token, $subject, $entity) = $this->setupMockObjects(); + + // Setup mocks for voteOnAttribute method + $this->setupMocksForVoteOnAttribute($user, $token, true, $entity, true); + + // The voteOnAttribute method should return True when workflow is allowed + $attributeSee = StoredObjectRoleEnum::SEE; + $attributeEdit = StoredObjectRoleEnum::EDIT; + $this->assertTrue($this->voter->voteOnAttribute($attributeSee, $subject, $token)); + } + + public function testVoteOnAttributeNotAllowed(): void + { + list($user, $token, $subject, $entity) = $this->setupMockObjects(); + + // Setup mocks for voteOnAttribute method where isGranted() returns false + $this->setupMocksForVoteOnAttribute($user, $token, false, $entity, true); + + // The voteOnAttribute method should return True when workflow is allowed + $attributeSee = StoredObjectRoleEnum::SEE; + $attributeEdit = StoredObjectRoleEnum::EDIT; + $this->assertTrue($this->voter->voteOnAttribute($attributeSee, $subject, $token)); + } + + public function testVoteOnAttributeWhenBlockedByWorkflow(): void + { + list($user, $token, $subject, $entity) = $this->setupMockObjects(); + + // Setup mocks for voteOnAttribute method + $this->setupMocksForVoteOnAttribute($user, $token, $subject, $entity, false); + + // Test voteOnAttribute method + $attribute = StoredObjectRoleEnum::SEE; + $result = $this->voter->voteOnAttribute($attribute, $subject, $token); + + // Assert that access is denied when workflow is not allowed + $this->assertFalse($result); + } +}