From fb88bfc79aaed2bfbd67105ba6b5978317da4ac4 Mon Sep 17 00:00:00 2001 From: Ivan Nikolaev Date: Thu, 19 Jul 2018 20:09:12 +0300 Subject: [PATCH 1/2] [FrameworkBundle] fixed guard event names for transitions --- .../Resources/config/schema/symfony-1.0.xsd | 1 + .../php/workflow_with_guard_expression.php | 51 +++++++++++ .../xml/workflow_with_guard_expression.xml | 48 +++++++++++ .../yml/workflow_with_guard_expression.yml | 35 ++++++++ .../FrameworkExtensionTest.php | 86 +++++++++++++++++-- .../EventListener/GuardExpression.php | 37 ++++++++ .../Workflow/EventListener/GuardListener.php | 25 +++++- .../Tests/EventListener/GuardListenerTest.php | 46 +++++++++- 8 files changed, 315 insertions(+), 14 deletions(-) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_guard_expression.php create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_guard_expression.yml create mode 100644 src/Symfony/Component/Workflow/EventListener/GuardExpression.php diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd index 1feb77bba863..adbf4e5c574e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd @@ -300,6 +300,7 @@ + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_guard_expression.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_guard_expression.php new file mode 100644 index 000000000000..89c86339afe1 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_guard_expression.php @@ -0,0 +1,51 @@ +loadFromExtension('framework', array( + 'workflows' => array( + 'article' => array( + 'type' => 'workflow', + 'marking_store' => array( + 'type' => 'multiple_state', + ), + 'supports' => array( + FrameworkExtensionTest::class, + ), + 'initial_place' => 'draft', + 'places' => array( + 'draft', + 'wait_for_journalist', + 'approved_by_journalist', + 'wait_for_spellchecker', + 'approved_by_spellchecker', + 'published', + ), + 'transitions' => array( + 'request_review' => array( + 'from' => 'draft', + 'to' => array('wait_for_journalist', 'wait_for_spellchecker'), + ), + 'journalist_approval' => array( + 'from' => 'wait_for_journalist', + 'to' => 'approved_by_journalist', + ), + 'spellchecker_approval' => array( + 'from' => 'wait_for_spellchecker', + 'to' => 'approved_by_spellchecker', + ), + 'publish' => array( + 'from' => array('approved_by_journalist', 'approved_by_spellchecker'), + 'to' => 'published', + 'guard' => '!!true', + ), + 'publish_editor_in_chief' => array( + 'name' => 'publish', + 'from' => 'draft', + 'to' => 'published', + 'guard' => '!!false', + ), + ), + ), + ), +)); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml new file mode 100644 index 000000000000..cf129e45c33c --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml @@ -0,0 +1,48 @@ + + + + + + + + a + a + + Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest + + + + + + + + draft + wait_for_journalist + wait_for_spellchecker + + + wait_for_journalist + approved_by_journalist + + + wait_for_spellchecker + approved_by_spellchecker + + + approved_by_journalist + approved_by_spellchecker + published + !!true + + + draft + published + !!false + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_guard_expression.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_guard_expression.yml new file mode 100644 index 000000000000..458cb4ae1ee7 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_guard_expression.yml @@ -0,0 +1,35 @@ +framework: + workflows: + article: + type: workflow + marking_store: + type: multiple_state + supports: + - Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest + initial_place: draft + places: + - draft + - wait_for_journalist + - approved_by_journalist + - wait_for_spellchecker + - approved_by_spellchecker + - published + transitions: + request_review: + from: [draft] + to: [wait_for_journalist, wait_for_spellchecker] + journalist_approval: + from: [wait_for_journalist] + to: [approved_by_journalist] + spellchecker_approval: + from: [wait_for_spellchecker] + to: [approved_by_spellchecker] + publish: + from: [approved_by_journalist, approved_by_spellchecker] + to: [published] + guard: "!!true" + publish_editor_in_chief: + name: publish + from: [draft] + to: [published] + guard: "!!false" diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 38420ac1e767..ae6dc7a75ace 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -302,14 +302,84 @@ public function testWorkflowMultipleTransitionsWithSameName() $this->assertCount(5, $transitions); - $this->assertSame('request_review', $transitions[0]->getArgument(0)); - $this->assertSame('journalist_approval', $transitions[1]->getArgument(0)); - $this->assertSame('spellchecker_approval', $transitions[2]->getArgument(0)); - $this->assertSame('publish', $transitions[3]->getArgument(0)); - $this->assertSame('publish', $transitions[4]->getArgument(0)); - - $this->assertSame(array('approved_by_journalist', 'approved_by_spellchecker'), $transitions[3]->getArgument(1)); - $this->assertSame(array('draft'), $transitions[4]->getArgument(1)); + $this->assertSame('workflow.article.transition.0', (string) $transitions[0]); + $this->assertSame(array( + 'request_review', + array( + 'draft', + ), + array( + 'wait_for_journalist', 'wait_for_spellchecker', + ), + ), $container->getDefinition($transitions[0])->getArguments()); + + $this->assertSame('workflow.article.transition.1', (string) $transitions[1]); + $this->assertSame(array( + 'journalist_approval', + array( + 'wait_for_journalist', + ), + array( + 'approved_by_journalist', + ), + ), $container->getDefinition($transitions[1])->getArguments()); + + $this->assertSame('workflow.article.transition.2', (string) $transitions[2]); + $this->assertSame(array( + 'spellchecker_approval', + array( + 'wait_for_spellchecker', + ), + array( + 'approved_by_spellchecker', + ), + ), $container->getDefinition($transitions[2])->getArguments()); + + $this->assertSame('workflow.article.transition.3', (string) $transitions[3]); + $this->assertSame(array( + 'publish', + array( + 'approved_by_journalist', + 'approved_by_spellchecker', + ), + array( + 'published', + ), + ), $container->getDefinition($transitions[3])->getArguments()); + + $this->assertSame('workflow.article.transition.4', (string) $transitions[4]); + $this->assertSame(array( + 'publish', + array( + 'draft', + ), + array( + 'published', + ), + ), $container->getDefinition($transitions[4])->getArguments()); + } + + public function testGuardExpressions() + { + $container = $this->createContainerFromFile('workflow_with_guard_expression'); + + $this->assertTrue($container->hasDefinition('workflow.article.listener.guard'), 'Workflow guard listener is registered as a service'); + $this->assertTrue($container->hasParameter('workflow.has_guard_listeners'), 'Workflow guard listeners parameter exists'); + $this->assertTrue(true === $container->getParameter('workflow.has_guard_listeners'), 'Workflow guard listeners parameter is enabled'); + $guardDefinition = $container->getDefinition('workflow.article.listener.guard'); + $this->assertSame(array( + array( + 'event' => 'workflow.article.guard.publish', + 'method' => 'onTransition', + ), + ), $guardDefinition->getTag('kernel.event_listener')); + $guardsConfiguration = $guardDefinition->getArgument(0); + $this->assertTrue(1 === \count($guardsConfiguration), 'Workflow guard configuration contains one element per transition name'); + $transitionGuardExpressions = $guardsConfiguration['workflow.article.guard.publish']; + $this->assertSame('workflow.article.transition.3', (string) $transitionGuardExpressions[0]->getArgument(0)); + $this->assertSame('!!true', $transitionGuardExpressions[0]->getArgument(1)); + $this->assertSame('workflow.article.transition.4', (string) $transitionGuardExpressions[1]->getArgument(0)); + $this->assertSame('!!false', $transitionGuardExpressions[1]->getArgument(1)); } public function testWorkflowServicesCanBeEnabled() diff --git a/src/Symfony/Component/Workflow/EventListener/GuardExpression.php b/src/Symfony/Component/Workflow/EventListener/GuardExpression.php new file mode 100644 index 000000000000..cf3b8c7e1869 --- /dev/null +++ b/src/Symfony/Component/Workflow/EventListener/GuardExpression.php @@ -0,0 +1,37 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Workflow\EventListener; + +use Symfony\Component\Workflow\Transition; + +class GuardExpression +{ + private $transition; + + private $expression; + + public function getTransition(): Transition + { + return $this->transition; + } + + public function getExpression(): string + { + return $this->expression; + } + + public function __construct(Transition $transition, string $expression) + { + $this->transition = $transition; + $this->expression = $expression; + } +} diff --git a/src/Symfony/Component/Workflow/EventListener/GuardListener.php b/src/Symfony/Component/Workflow/EventListener/GuardListener.php index 893f304e4783..4f1c229e51b6 100644 --- a/src/Symfony/Component/Workflow/EventListener/GuardListener.php +++ b/src/Symfony/Component/Workflow/EventListener/GuardListener.php @@ -18,6 +18,7 @@ use Symfony\Component\Validator\Validator\ValidatorInterface; use Symfony\Component\Workflow\Event\GuardEvent; use Symfony\Component\Workflow\Exception\InvalidTokenConfigurationException; +use Symfony\Component\Workflow\TransitionBlocker; /** * @author Grégoire Pineau @@ -32,7 +33,7 @@ class GuardListener private $roleHierarchy; private $validator; - public function __construct($configuration, ExpressionLanguage $expressionLanguage, TokenStorageInterface $tokenStorage, AuthorizationCheckerInterface $authenticationChecker, AuthenticationTrustResolverInterface $trustResolver, RoleHierarchyInterface $roleHierarchy = null, ValidatorInterface $validator = null) + public function __construct(array $configuration, ExpressionLanguage $expressionLanguage, TokenStorageInterface $tokenStorage, AuthorizationCheckerInterface $authenticationChecker, AuthenticationTrustResolverInterface $trustResolver, RoleHierarchyInterface $roleHierarchy = null, ValidatorInterface $validator = null) { $this->configuration = $configuration; $this->expressionLanguage = $expressionLanguage; @@ -49,13 +50,29 @@ public function onTransition(GuardEvent $event, $eventName) return; } - if (!$this->expressionLanguage->evaluate($this->configuration[$eventName], $this->getVariables($event))) { - $event->setBlocked(true); + $eventConfiguration = (array) $this->configuration[$eventName]; + foreach ($eventConfiguration as $guard) { + if ($guard instanceof GuardExpression) { + if ($guard->getTransition() !== $event->getTransition()) { + continue; + } + $this->validateGuardExpression($event, $guard->getExpression()); + } else { + $this->validateGuardExpression($event, $guard); + } + } + } + + private function validateGuardExpression(GuardEvent $event, string $expression) + { + if (!$this->expressionLanguage->evaluate($expression, $this->getVariables($event))) { + $blocker = TransitionBlocker::createBlockedByExpressionGuardListener($expression); + $event->addTransitionBlocker($blocker); } } // code should be sync with Symfony\Component\Security\Core\Authorization\Voter\ExpressionVoter - private function getVariables(GuardEvent $event) + private function getVariables(GuardEvent $event): array { $token = $this->tokenStorage->getToken(); diff --git a/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php b/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php index e224835a0764..f4c646ade345 100644 --- a/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php +++ b/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php @@ -11,6 +11,7 @@ use Symfony\Component\Validator\Validator\ValidatorInterface; use Symfony\Component\Workflow\Event\GuardEvent; use Symfony\Component\Workflow\EventListener\ExpressionLanguage; +use Symfony\Component\Workflow\EventListener\GuardExpression; use Symfony\Component\Workflow\EventListener\GuardListener; use Symfony\Component\Workflow\Marking; use Symfony\Component\Workflow\Transition; @@ -20,12 +21,17 @@ class GuardListenerTest extends TestCase private $authenticationChecker; private $validator; private $listener; + private $transition; protected function setUp() { $configuration = array( 'test_is_granted' => 'is_granted("something")', 'test_is_valid' => 'is_valid(subject)', + 'test_expression' => array( + new GuardExpression($this->getTransition(true), '!is_valid(subject)'), + new GuardExpression($this->getTransition(true), 'is_valid(subject)'), + ), ); $expressionLanguage = new ExpressionLanguage(); $token = $this->getMockBuilder(TokenInterface::class)->getMock(); @@ -96,11 +102,38 @@ public function testWithValidatorSupportedEventAndAccept() $this->assertFalse($event->isBlocked()); } - private function createEvent() + public function testWithGuardExpressionWithNotSupportedTransition() + { + $event = $this->createEvent(true); + $this->configureValidator(false, false); + $this->listener->onTransition($event, 'test_expression'); + + $this->assertFalse($event->isBlocked()); + } + + public function testWithGuardExpressionWithSupportedTransition() + { + $event = $this->createEvent(); + $this->configureValidator(true, true); + $this->listener->onTransition($event, 'test_expression'); + + $this->assertFalse($event->isBlocked()); + } + + public function testGuardExpressionBlocks() + { + $event = $this->createEvent(); + $this->configureValidator(true, false); + $this->listener->onTransition($event, 'test_expression'); + + $this->assertTrue($event->isBlocked()); + } + + private function createEvent($newTransition = false) { $subject = new \stdClass(); $subject->marking = new Marking(); - $transition = new Transition('name', 'from', 'to'); + $transition = $this->getTransition($newTransition); return new GuardEvent($subject, $subject->marking, $transition); } @@ -140,4 +173,13 @@ private function configureValidator($isUsed, $valid = true) ->willReturn($valid ? array() : array('a violation')) ; } + + private function getTransition($new = false) + { + if ($new || !$this->transition) { + $this->transition = new Transition('name', 'from', 'to'); + } + + return $this->transition; + } } From 83dc473dd6dc29316546c3c0a9ea834adf1d93d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Thu, 8 Nov 2018 14:02:15 +0100 Subject: [PATCH 2/2] [FrameworkBundle] fixed guard event names for transitions --- .../FrameworkExtension.php | 58 +++++++++++++------ .../xml/workflow_with_guard_expression.xml | 12 ++-- .../EventListener/GuardExpression.php | 17 +++--- .../Workflow/EventListener/GuardListener.php | 8 +-- .../Tests/EventListener/GuardListenerTest.php | 31 ++++------ 5 files changed, 69 insertions(+), 57 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 5b6c8b447ed7..9ec02f07228a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -602,15 +602,44 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ @trigger_error(sprintf('The "type" option of the "framework.workflows.%s" configuration entry must be defined since Symfony 3.3. The default value will be "state_machine" in Symfony 4.0.', $name), E_USER_DEPRECATED); } $type = $workflow['type']; + $workflowId = sprintf('%s.%s', $type, $name); + // Create transitions $transitions = array(); + $guardsConfiguration = array(); + // Global transition counter per workflow + $transitionCounter = 0; foreach ($workflow['transitions'] as $transition) { if ('workflow' === $type) { - $transitions[] = new Definition(Workflow\Transition::class, array($transition['name'], $transition['from'], $transition['to'])); + $transitionDefinition = new Definition(Workflow\Transition::class, array($transition['name'], $transition['from'], $transition['to'])); + $transitionDefinition->setPublic(false); + $transitionId = sprintf('%s.transition.%s', $workflowId, $transitionCounter++); + $container->setDefinition($transitionId, $transitionDefinition); + $transitions[] = new Reference($transitionId); + if (isset($transition['guard'])) { + $configuration = new Definition(Workflow\EventListener\GuardExpression::class); + $configuration->addArgument(new Reference($transitionId)); + $configuration->addArgument($transition['guard']); + $configuration->setPublic(false); + $eventName = sprintf('workflow.%s.guard.%s', $name, $transition['name']); + $guardsConfiguration[$eventName][] = $configuration; + } } elseif ('state_machine' === $type) { foreach ($transition['from'] as $from) { foreach ($transition['to'] as $to) { - $transitions[] = new Definition(Workflow\Transition::class, array($transition['name'], $from, $to)); + $transitionDefinition = new Definition(Workflow\Transition::class, array($transition['name'], $from, $to)); + $transitionDefinition->setPublic(false); + $transitionId = sprintf('%s.transition.%s', $workflowId, $transitionCounter++); + $container->setDefinition($transitionId, $transitionDefinition); + $transitions[] = new Reference($transitionId); + if (isset($transition['guard'])) { + $configuration = new Definition(Workflow\EventListener\GuardExpression::class); + $configuration->addArgument(new Reference($transitionId)); + $configuration->addArgument($transition['guard']); + $configuration->setPublic(false); + $eventName = sprintf('workflow.%s.guard.%s', $name, $transition['name']); + $guardsConfiguration[$eventName][] = $configuration; + } } } } @@ -641,7 +670,6 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ } // Create Workflow - $workflowId = sprintf('%s.%s', $type, $name); $workflowDefinition = new ChildDefinition(sprintf('%s.abstract', $type)); $workflowDefinition->replaceArgument(0, new Reference(sprintf('%s.definition', $workflowId))); if (isset($markingStoreDefinition)) { @@ -677,16 +705,7 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ } // Add Guard Listener - $guard = new Definition(Workflow\EventListener\GuardListener::class); - $guard->setPrivate(true); - $configuration = array(); - foreach ($workflow['transitions'] as $config) { - $transitionName = $config['name']; - - if (!isset($config['guard'])) { - continue; - } - + if ($guardsConfiguration) { if (!class_exists(ExpressionLanguage::class)) { throw new LogicException('Cannot guard workflows as the ExpressionLanguage component is not installed. Try running "composer require symfony/expression-language".'); } @@ -695,13 +714,11 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ throw new LogicException('Cannot guard workflows as the Security component is not installed. Try running "composer require symfony/security".'); } - $eventName = sprintf('workflow.%s.guard.%s', $name, $transitionName); - $guard->addTag('kernel.event_listener', array('event' => $eventName, 'method' => 'onTransition')); - $configuration[$eventName] = $config['guard']; - } - if ($configuration) { + $guard = new Definition(Workflow\EventListener\GuardListener::class); + $guard->setPrivate(true); + $guard->setArguments(array( - $configuration, + $guardsConfiguration, new Reference('workflow.security.expression_language'), new Reference('security.token_storage'), new Reference('security.authorization_checker'), @@ -709,6 +726,9 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ new Reference('security.role_hierarchy'), new Reference('validator', ContainerInterface::NULL_ON_INVALID_REFERENCE), )); + foreach ($guardsConfiguration as $eventName => $config) { + $guard->addTag('kernel.event_listener', array('event' => $eventName, 'method' => 'onTransition')); + } $container->setDefinition(sprintf('%s.listener.guard', $workflowId), $guard); $container->setParameter('workflow.has_guard_listeners', true); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml index cf129e45c33c..a5124d1fe777 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml @@ -13,12 +13,12 @@ a Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest - - - - - - + draft + wait_for_journalist + approved_by_journalist + wait_for_spellchecker + approved_by_spellchecker + published draft wait_for_journalist diff --git a/src/Symfony/Component/Workflow/EventListener/GuardExpression.php b/src/Symfony/Component/Workflow/EventListener/GuardExpression.php index cf3b8c7e1869..09ab15086bdf 100644 --- a/src/Symfony/Component/Workflow/EventListener/GuardExpression.php +++ b/src/Symfony/Component/Workflow/EventListener/GuardExpression.php @@ -19,19 +19,22 @@ class GuardExpression private $expression; - public function getTransition(): Transition + /** + * @param string $expression + */ + public function __construct(Transition $transition, $expression) { - return $this->transition; + $this->transition = $transition; + $this->expression = $expression; } - public function getExpression(): string + public function getTransition() { - return $this->expression; + return $this->transition; } - public function __construct(Transition $transition, string $expression) + public function getExpression() { - $this->transition = $transition; - $this->expression = $expression; + return $this->expression; } } diff --git a/src/Symfony/Component/Workflow/EventListener/GuardListener.php b/src/Symfony/Component/Workflow/EventListener/GuardListener.php index 4f1c229e51b6..4d3cfac57e7d 100644 --- a/src/Symfony/Component/Workflow/EventListener/GuardListener.php +++ b/src/Symfony/Component/Workflow/EventListener/GuardListener.php @@ -18,7 +18,6 @@ use Symfony\Component\Validator\Validator\ValidatorInterface; use Symfony\Component\Workflow\Event\GuardEvent; use Symfony\Component\Workflow\Exception\InvalidTokenConfigurationException; -use Symfony\Component\Workflow\TransitionBlocker; /** * @author Grégoire Pineau @@ -63,16 +62,15 @@ public function onTransition(GuardEvent $event, $eventName) } } - private function validateGuardExpression(GuardEvent $event, string $expression) + private function validateGuardExpression(GuardEvent $event, $expression) { if (!$this->expressionLanguage->evaluate($expression, $this->getVariables($event))) { - $blocker = TransitionBlocker::createBlockedByExpressionGuardListener($expression); - $event->addTransitionBlocker($blocker); + $event->setBlocked(true); } } // code should be sync with Symfony\Component\Security\Core\Authorization\Voter\ExpressionVoter - private function getVariables(GuardEvent $event): array + private function getVariables(GuardEvent $event) { $token = $this->tokenStorage->getToken(); diff --git a/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php b/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php index f4c646ade345..8686d74cf6ca 100644 --- a/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php +++ b/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php @@ -21,16 +21,16 @@ class GuardListenerTest extends TestCase private $authenticationChecker; private $validator; private $listener; - private $transition; + private $configuration; protected function setUp() { - $configuration = array( + $this->configuration = array( 'test_is_granted' => 'is_granted("something")', 'test_is_valid' => 'is_valid(subject)', 'test_expression' => array( - new GuardExpression($this->getTransition(true), '!is_valid(subject)'), - new GuardExpression($this->getTransition(true), 'is_valid(subject)'), + new GuardExpression(new Transition('name', 'from', 'to'), '!is_valid(subject)'), + new GuardExpression(new Transition('name', 'from', 'to'), 'is_valid(subject)'), ), ); $expressionLanguage = new ExpressionLanguage(); @@ -41,7 +41,7 @@ protected function setUp() $this->authenticationChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock(); $trustResolver = $this->getMockBuilder(AuthenticationTrustResolverInterface::class)->getMock(); $this->validator = $this->getMockBuilder(ValidatorInterface::class)->getMock(); - $this->listener = new GuardListener($configuration, $expressionLanguage, $tokenStorage, $this->authenticationChecker, $trustResolver, null, $this->validator); + $this->listener = new GuardListener($this->configuration, $expressionLanguage, $tokenStorage, $this->authenticationChecker, $trustResolver, null, $this->validator); } protected function tearDown() @@ -104,8 +104,8 @@ public function testWithValidatorSupportedEventAndAccept() public function testWithGuardExpressionWithNotSupportedTransition() { - $event = $this->createEvent(true); - $this->configureValidator(false, false); + $event = $this->createEvent(); + $this->configureValidator(false); $this->listener->onTransition($event, 'test_expression'); $this->assertFalse($event->isBlocked()); @@ -113,7 +113,7 @@ public function testWithGuardExpressionWithNotSupportedTransition() public function testWithGuardExpressionWithSupportedTransition() { - $event = $this->createEvent(); + $event = $this->createEvent($this->configuration['test_expression'][1]->getTransition()); $this->configureValidator(true, true); $this->listener->onTransition($event, 'test_expression'); @@ -122,18 +122,18 @@ public function testWithGuardExpressionWithSupportedTransition() public function testGuardExpressionBlocks() { - $event = $this->createEvent(); + $event = $this->createEvent($this->configuration['test_expression'][1]->getTransition()); $this->configureValidator(true, false); $this->listener->onTransition($event, 'test_expression'); $this->assertTrue($event->isBlocked()); } - private function createEvent($newTransition = false) + private function createEvent(Transition $transition = null) { $subject = new \stdClass(); $subject->marking = new Marking(); - $transition = $this->getTransition($newTransition); + $transition = $transition ?: new Transition('name', 'from', 'to'); return new GuardEvent($subject, $subject->marking, $transition); } @@ -173,13 +173,4 @@ private function configureValidator($isUsed, $valid = true) ->willReturn($valid ? array() : array('a violation')) ; } - - private function getTransition($new = false) - { - if ($new || !$this->transition) { - $this->transition = new Transition('name', 'from', 'to'); - } - - return $this->transition; - } }