From bb292776d7d6f254f2b4ace2c6ddf087c79de524 Mon Sep 17 00:00:00 2001 From: Vytautas Stankus Date: Wed, 26 Oct 2016 15:32:30 +0300 Subject: [PATCH 1/3] stop propagation only after MvcEvent::EVENT_DISPATCH_ERROR because only one listener would be triggered --- src/ZfcRbac/Guard/AbstractGuard.php | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/ZfcRbac/Guard/AbstractGuard.php b/src/ZfcRbac/Guard/AbstractGuard.php index 3e36fa0..4cd86f6 100644 --- a/src/ZfcRbac/Guard/AbstractGuard.php +++ b/src/ZfcRbac/Guard/AbstractGuard.php @@ -68,18 +68,13 @@ public function onResult(MvcEvent $event) 403 )); - $event->stopPropagation(true); - $application = $event->getApplication(); $eventManager = $application->getEventManager(); - if (method_exists($eventManager, 'triggerEvent')) { - // ZF3 EventManager - $event->setName(MvcEvent::EVENT_DISPATCH_ERROR); - $eventManager->triggerEvent($event); - } else { - // ZF2 EventManager - $eventManager->trigger(MvcEvent::EVENT_DISPATCH_ERROR, $event); - } + $event->setName(MvcEvent::EVENT_DISPATCH_ERROR); + $eventManager->triggerEvent($event); + + // just in case + $event->stopPropagation(true); } } From 0d093924b47bca56164de28692630be11f47b1d9 Mon Sep 17 00:00:00 2001 From: Vytautas Stankus Date: Wed, 26 Oct 2016 15:53:48 +0300 Subject: [PATCH 2/3] fixed tests --- tests/ZfcRbacTest/Guard/ControllerGuardTest.php | 14 ++++---------- .../Guard/ControllerPermissionsGuardTest.php | 14 ++++---------- tests/ZfcRbacTest/Guard/RouteGuardTest.php | 14 ++++---------- .../Guard/RoutePermissionsGuardTest.php | 14 ++++---------- 4 files changed, 16 insertions(+), 40 deletions(-) diff --git a/tests/ZfcRbacTest/Guard/ControllerGuardTest.php b/tests/ZfcRbacTest/Guard/ControllerGuardTest.php index 4123489..50c996f 100644 --- a/tests/ZfcRbacTest/Guard/ControllerGuardTest.php +++ b/tests/ZfcRbacTest/Guard/ControllerGuardTest.php @@ -488,21 +488,15 @@ public function testProperlySetUnauthorizedAndTriggerEventOnUnauthorization() ]); $application = $this->getMock('Zend\Mvc\Application', [], [], '', false); - $eventManager = $this->getMock('Zend\EventManager\EventManagerInterface'); + $eventManager = $this->getMock('Zend\EventManager\EventManager'); $application->expects($this->once()) ->method('getEventManager') ->will($this->returnValue($eventManager)); - if (method_exists($eventManager, 'triggerEvent')) { - $eventManager->expects($this->once()) - ->method('triggerEvent') - ->with($event); - } else { - $eventManager->expects($this->once()) - ->method('trigger') - ->with(MvcEvent::EVENT_DISPATCH_ERROR); - } + $eventManager->expects($this->once()) + ->method('triggerEvent') + ->with($event); $routeMatch->setParam('controller', 'MyController'); $routeMatch->setParam('action', 'delete'); diff --git a/tests/ZfcRbacTest/Guard/ControllerPermissionsGuardTest.php b/tests/ZfcRbacTest/Guard/ControllerPermissionsGuardTest.php index 618882d..99377e7 100644 --- a/tests/ZfcRbacTest/Guard/ControllerPermissionsGuardTest.php +++ b/tests/ZfcRbacTest/Guard/ControllerPermissionsGuardTest.php @@ -475,21 +475,15 @@ public function testProperlySetUnauthorizedAndTriggerEventOnUnauthorization() ]); $application = $this->getMock('Zend\Mvc\Application', [], [], '', false); - $eventManager = $this->getMock('Zend\EventManager\EventManagerInterface'); + $eventManager = $this->getMock('Zend\EventManager\EventManager'); $application->expects($this->once()) ->method('getEventManager') ->will($this->returnValue($eventManager)); - if (method_exists($eventManager, 'triggerEvent')) { - $eventManager->expects($this->once()) - ->method('triggerEvent') - ->with($event); - } else { - $eventManager->expects($this->once()) - ->method('trigger') - ->with(MvcEvent::EVENT_DISPATCH_ERROR); - } + $eventManager->expects($this->once()) + ->method('triggerEvent') + ->with($event); $event->setRouteMatch($routeMatch); $event->setApplication($application); diff --git a/tests/ZfcRbacTest/Guard/RouteGuardTest.php b/tests/ZfcRbacTest/Guard/RouteGuardTest.php index c979247..9287d84 100644 --- a/tests/ZfcRbacTest/Guard/RouteGuardTest.php +++ b/tests/ZfcRbacTest/Guard/RouteGuardTest.php @@ -433,21 +433,15 @@ public function testProperlySetUnauthorizedAndTriggerEventOnUnauthorization() $routeMatch = $this->createRouteMatch(); $application = $this->getMock('Zend\Mvc\Application', [], [], '', false); - $eventManager = $this->getMock('Zend\EventManager\EventManagerInterface'); + $eventManager = $this->getMock('Zend\EventManager\EventManager'); $application->expects($this->once()) ->method('getEventManager') ->will($this->returnValue($eventManager)); - if (method_exists($eventManager, 'triggerEvent')) { - $eventManager->expects($this->once()) - ->method('triggerEvent') - ->with($event); - } else { - $eventManager->expects($this->once()) - ->method('trigger') - ->with(MvcEvent::EVENT_DISPATCH_ERROR); - } + $eventManager->expects($this->once()) + ->method('triggerEvent') + ->with($event); $routeMatch->setMatchedRouteName('adminRoute'); $event->setRouteMatch($routeMatch); diff --git a/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php b/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php index 45f837b..445782b 100644 --- a/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php +++ b/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php @@ -410,7 +410,7 @@ public function testProperlyFillEventOnAuthorization() public function testProperlySetUnauthorizedAndTriggerEventOnUnauthorization() { - $eventManager = $this->getMock('Zend\EventManager\EventManagerInterface'); + $eventManager = $this->getMock('Zend\EventManager\EventManager'); $application = $this->getMock('Zend\Mvc\Application', [], [], '', false); $application->expects($this->once()) @@ -424,15 +424,9 @@ public function testProperlySetUnauthorizedAndTriggerEventOnUnauthorization() $event->setRouteMatch($routeMatch); $event->setApplication($application); - if (method_exists($eventManager, 'triggerEvent')) { - $eventManager->expects($this->once()) - ->method('triggerEvent') - ->with($event); - } else { - $eventManager->expects($this->once()) - ->method('trigger') - ->with(MvcEvent::EVENT_DISPATCH_ERROR); - } + $eventManager->expects($this->once()) + ->method('triggerEvent') + ->with($event); $authorizationService = $this->getMock('ZfcRbac\Service\AuthorizationServiceInterface', [], [], '', false); $authorizationService->expects($this->once()) From a3afdac4a2dcdd4bf8eb169275499c9266d4344b Mon Sep 17 00:00:00 2001 From: Vytautas Stankus Date: Wed, 26 Oct 2016 16:58:05 +0300 Subject: [PATCH 3/3] Added test to confirm this fix --- tests/ZfcRbacTest/Asset/DummyGuard.php | 35 +++++++++++ tests/ZfcRbacTest/Guard/AbstractGuardTest.php | 63 +++++++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 tests/ZfcRbacTest/Asset/DummyGuard.php create mode 100644 tests/ZfcRbacTest/Guard/AbstractGuardTest.php diff --git a/tests/ZfcRbacTest/Asset/DummyGuard.php b/tests/ZfcRbacTest/Asset/DummyGuard.php new file mode 100644 index 0000000..7f550bb --- /dev/null +++ b/tests/ZfcRbacTest/Asset/DummyGuard.php @@ -0,0 +1,35 @@ +prophesize(Application::class); + $application->getEventManager()->willReturn($eventManager); + + $event = new MvcEvent(); + $event->setApplication($application->reveal()); + + $guard = new DummyGuard(); + $guard->attach($eventManager); + + $eventManager->attach(MvcEvent::EVENT_DISPATCH_ERROR, function (MvcEvent $event) { + $event->setParam('first-listener', true); + }); + $eventManager->attach(MvcEvent::EVENT_DISPATCH_ERROR, function (MvcEvent $event) { + $event->setParam('second-listener', true); + }); + + // attach listener with lower priority than DummyGuard + $eventManager->attach(MvcEvent::EVENT_ROUTE, function (MvcEvent $event) { + $this->fail('should not be called, because guard should stop propagation'); + }, DummyGuard::EVENT_PRIORITY - 1); + + $event->setName(MvcEvent::EVENT_ROUTE); + $eventManager->triggerEvent($event); + + $this->assertTrue($event->getParam('first-listener')); + $this->assertTrue($event->getParam('second-listener')); + $this->assertTrue($event->propagationIsStopped()); + } +}