From a71ba784782b87317694add148f8e7434767c6f7 Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Thu, 17 May 2018 18:15:49 +0200 Subject: [PATCH] [Security][SecurityBundle] FirewallMap/FirewallContext deprecations --- UPGRADE-4.2.md | 8 ++++ UPGRADE-5.0.md | 4 ++ .../Security/FirewallContext.php | 1 + .../SecurityDataCollectorTest.php | 2 +- .../Debug/TraceableFirewallListenerTest.php | 2 +- .../Tests/Security/FirewallContextTest.php | 9 +++++ .../Component/Security/Http/Firewall.php | 8 +++- .../Security/Http/FirewallMapInterface.php | 5 ++- .../Security/Http/Tests/FirewallTest.php | 37 ++++++++++++++++++- 9 files changed, 70 insertions(+), 6 deletions(-) diff --git a/UPGRADE-4.2.md b/UPGRADE-4.2.md index 23982ef159b3..2bac6f514b7a 100644 --- a/UPGRADE-4.2.md +++ b/UPGRADE-4.2.md @@ -5,3 +5,11 @@ Security -------- * Using the `has_role()` function in security expressions is deprecated, use the `is_granted()` function instead. + * Not returning an array of 3 elements from `FirewallMapInterface::getListeners()` is deprecated, the 3rd element + must be an instance of `LogoutListener` or `null`. + +SecurityBundle +-------------- + + * Passing a `FirewallConfig` instance as 3rd argument to the `FirewallContext` constructor is deprecated, + pass a `LogoutListener` instance instead. diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md index 61b7237b4492..48be0e094a65 100644 --- a/UPGRADE-5.0.md +++ b/UPGRADE-5.0.md @@ -78,6 +78,8 @@ Security * The `ContextListener::setLogoutOnUserChange()` method has been removed. * The `Symfony\Component\Security\Core\User\AdvancedUserInterface` has been removed. * The `ExpressionVoter::addExpressionLanguageProvider()` method has been removed. + * The `FirewallMapInterface::getListeners()` method must return an array of 3 elements, + the 3rd one must be either a `LogoutListener` instance or `null`. SecurityBundle -------------- @@ -85,6 +87,8 @@ SecurityBundle * The `logout_on_user_change` firewall option has been removed. * The `switch_user.stateless` firewall option has been removed. * The `SecurityUserValueResolver` class has been removed. + * Passing a `FirewallConfig` instance as 3rd argument to the `FirewallContext` constructor + now throws a `\TypeError`, pass a `LogoutListener` instance instead. Translation ----------- diff --git a/src/Symfony/Bundle/SecurityBundle/Security/FirewallContext.php b/src/Symfony/Bundle/SecurityBundle/Security/FirewallContext.php index a3b7f1540691..ac0d1f240684 100644 --- a/src/Symfony/Bundle/SecurityBundle/Security/FirewallContext.php +++ b/src/Symfony/Bundle/SecurityBundle/Security/FirewallContext.php @@ -36,6 +36,7 @@ public function __construct(iterable $listeners, ExceptionListener $exceptionLis $this->exceptionListener = $exceptionListener; if ($logoutListener instanceof FirewallConfig) { $this->config = $logoutListener; + @trigger_error(sprintf('Passing an instance of %s as 3rd argument to %s() is deprecated since Symfony 4.2. Pass a %s instance instead.', FirewallConfig::class, __METHOD__, LogoutListener::class), E_USER_DEPRECATED); } elseif (null === $logoutListener || $logoutListener instanceof LogoutListener) { $this->logoutListener = $logoutListener; $this->config = $config; diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php index 548b1939fc25..0ceacd22468b 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php @@ -208,7 +208,7 @@ public function testGetListeners() ->expects($this->once()) ->method('getListeners') ->with($request) - ->willReturn(array(array($listener), null)); + ->willReturn(array(array($listener), null, null)); $firewall = new TraceableFirewallListener($firewallMap, new EventDispatcher(), new LogoutUrlGenerator()); $firewall->onKernelRequest($event); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Debug/TraceableFirewallListenerTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Debug/TraceableFirewallListenerTest.php index 287ba531f403..829d1f11e67d 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Debug/TraceableFirewallListenerTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Debug/TraceableFirewallListenerTest.php @@ -51,7 +51,7 @@ public function testOnKernelRequestRecordsListeners() ->expects($this->once()) ->method('getListeners') ->with($request) - ->willReturn(array(array($listener), null)); + ->willReturn(array(array($listener), null, null)); $firewall = new TraceableFirewallListener($firewallMap, new EventDispatcher(), new LogoutUrlGenerator()); $firewall->onKernelRequest($event); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Security/FirewallContextTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Security/FirewallContextTest.php index 520a129716f4..129c72fab910 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Security/FirewallContextTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Security/FirewallContextTest.php @@ -40,6 +40,15 @@ public function testGetters() $this->assertEquals($config, $context->getConfig()); } + /** + * @group legacy + * @expectedDeprecation Passing an instance of Symfony\Bundle\SecurityBundle\Security\FirewallConfig as 3rd argument to Symfony\Bundle\SecurityBundle\Security\FirewallContext::__construct() is deprecated since Symfony 4.2. Pass a Symfony\Component\Security\Http\Firewall\LogoutListener instance instead. + */ + public function testFirewallConfigAs3rdConstructorArgument() + { + new FirewallContext(array(), $this->getExceptionListenerMock(), new FirewallConfig('main', 'user_checker', 'request_matcher')); + } + private function getExceptionListenerMock() { return $this diff --git a/src/Symfony/Component/Security/Http/Firewall.php b/src/Symfony/Component/Security/Http/Firewall.php index 9bee596759bd..b533f547b643 100644 --- a/src/Symfony/Component/Security/Http/Firewall.php +++ b/src/Symfony/Component/Security/Http/Firewall.php @@ -16,6 +16,7 @@ use Symfony\Component\HttpKernel\Event\FinishRequestEvent; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\Security\Http\Firewall\LogoutListener; /** * Firewall uses a FirewallMap to register security listeners for the given @@ -49,9 +50,14 @@ public function onKernelRequest(GetResponseEvent $event) // register listeners for this firewall $listeners = $this->map->getListeners($event->getRequest()); + if (3 !== \count($listeners)) { + @trigger_error(sprintf('Not returning an array of 3 elements from %s::getListeners() is deprecated since Symfony 4.2, the 3rd element must be an instance of %s or null.', FirewallMapInterface::class, LogoutListener::class), E_USER_DEPRECATED); + $listeners[2] = null; + } + $authenticationListeners = $listeners[0]; $exceptionListener = $listeners[1]; - $logoutListener = isset($listeners[2]) ? $listeners[2] : null; + $logoutListener = $listeners[2]; if (null !== $exceptionListener) { $this->exceptionListeners[$event->getRequest()] = $exceptionListener; diff --git a/src/Symfony/Component/Security/Http/FirewallMapInterface.php b/src/Symfony/Component/Security/Http/FirewallMapInterface.php index ebdd498123a1..be7a78ccdb88 100644 --- a/src/Symfony/Component/Security/Http/FirewallMapInterface.php +++ b/src/Symfony/Component/Security/Http/FirewallMapInterface.php @@ -30,7 +30,10 @@ interface FirewallMapInterface * If there is no exception listener, the second element of the outer array * must be null. * - * @return array of the format array(array(AuthenticationListener), ExceptionListener) + * If there is no logout listener, the third element of the outer array + * must be null. + * + * @return array of the format array(array(AuthenticationListener), ExceptionListener, LogoutListener) */ public function getListeners(Request $request); } diff --git a/src/Symfony/Component/Security/Http/Tests/FirewallTest.php b/src/Symfony/Component/Security/Http/Tests/FirewallTest.php index bd475bb4e5b1..acc231dc9358 100644 --- a/src/Symfony/Component/Security/Http/Tests/FirewallTest.php +++ b/src/Symfony/Component/Security/Http/Tests/FirewallTest.php @@ -12,10 +12,14 @@ namespace Symfony\Component\Security\Http\Tests; use PHPUnit\Framework\TestCase; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\Security\Http\Firewall; +use Symfony\Component\Security\Http\Firewall\ExceptionListener; +use Symfony\Component\Security\Http\FirewallMapInterface; class FirewallTest extends TestCase { @@ -37,7 +41,7 @@ public function testOnKernelRequestRegistersExceptionListener() ->expects($this->once()) ->method('getListeners') ->with($this->equalTo($request)) - ->will($this->returnValue(array(array(), $listener))) + ->will($this->returnValue(array(array(), $listener, null))) ; $event = new GetResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $request, HttpKernelInterface::MASTER_REQUEST); @@ -66,7 +70,7 @@ public function testOnKernelRequestStopsWhenThereIsAResponse() $map ->expects($this->once()) ->method('getListeners') - ->will($this->returnValue(array(array($first, $second), null))) + ->will($this->returnValue(array(array($first, $second), null, null))) ; $event = $this->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent') @@ -107,4 +111,33 @@ public function testOnKernelRequestWithSubRequest() $this->assertFalse($event->hasResponse()); } + + /** + * @group legacy + * @expectedDeprecation Not returning an array of 3 elements from Symfony\Component\Security\Http\FirewallMapInterface::getListeners() is deprecated since Symfony 4.2, the 3rd element must be an instance of Symfony\Component\Security\Http\Firewall\LogoutListener or null. + */ + public function testMissingLogoutListener() + { + $dispatcher = $this->getMockBuilder(EventDispatcherInterface::class)->getMock(); + + $listener = $this->getMockBuilder(ExceptionListener::class)->disableOriginalConstructor()->getMock(); + $listener + ->expects($this->once()) + ->method('register') + ->with($this->equalTo($dispatcher)) + ; + + $request = new Request(); + + $map = $this->getMockBuilder(FirewallMapInterface::class)->getMock(); + $map + ->expects($this->once()) + ->method('getListeners') + ->with($this->equalTo($request)) + ->willReturn(array(array(), $listener)) + ; + + $firewall = new Firewall($map, $dispatcher); + $firewall->onKernelRequest(new GetResponseEvent($this->getMockBuilder(HttpKernelInterface::class)->getMock(), $request, HttpKernelInterface::MASTER_REQUEST)); + } }