[Security] Trigger a deprecation when a voter is missing the VoterInterface

This commit is contained in:
Iltar van der Berg 2017-05-04 09:35:02 +02:00 committed by Fabien Potencier
parent 243e4162ee
commit 9c253e1ff6
9 changed files with 199 additions and 9 deletions

8
UPGRADE-3.4.md Normal file
View File

@ -0,0 +1,8 @@
UPGRADE FROM 3.3 to 3.4
=======================
Security
--------
* Using voters that do not implement the `VoterInterface`is now deprecated in
the `AccessDecisionManager` and this functionality will be removed in 4.0.

View File

@ -1,6 +1,12 @@
CHANGELOG
=========
3.4.0
-----
* Tagging voters with the `security.voter` tag without implementing the
`VoterInterface` on the class is now deprecated and will be removed in 4.0.
3.3.0
-----

View File

@ -16,6 +16,7 @@ use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceTrait;
use Symfony\Component\DependencyInjection\Exception\LogicException;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
/**
* Adds all configured security voters to the access decision manager.
@ -40,6 +41,19 @@ class AddSecurityVotersPass implements CompilerPassInterface
throw new LogicException('No security voters found. You need to tag at least one with "security.voter"');
}
foreach ($voters as $voter) {
$class = $container->getDefinition((string) $voter)->getClass();
if (!is_a($class, VoterInterface::class, true)) {
@trigger_error(sprintf('Using a security.voter tag on a class without implementing the %1$s is deprecated as of 3.4 and will be removed in 4.0. Implement the %1$s instead.', VoterInterface::class), E_USER_DEPRECATED);
}
if (!method_exists($class, 'vote')) {
// in case the vote method is completely missing, to prevent exceptions when voting
throw new LogicException(sprintf('%s should implement the %s interface when used as voter.', $class, VoterInterface::class));
}
}
$adm = $container->getDefinition('security.access.decision_manager');
$adm->replaceArgument(0, new IteratorArgument($voters));
}

View File

@ -14,7 +14,11 @@ namespace Symfony\Bundle\SecurityBundle\Tests\DependencyInjection\Compiler;
use PHPUnit\Framework\TestCase;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Exception\LogicException;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
use Symfony\Component\Security\Core\Authorization\Voter\Voter;
use Symfony\Component\Security\Core\Tests\Authorization\Stub\VoterWithoutInterface;
class AddSecurityVotersPassTest extends TestCase
{
@ -25,7 +29,7 @@ class AddSecurityVotersPassTest extends TestCase
{
$container = new ContainerBuilder();
$container
->register('security.access.decision_manager', 'Symfony\Component\Security\Core\Authorization\AccessDecisionManager')
->register('security.access.decision_manager', AccessDecisionManager::class)
->addArgument(array())
;
@ -37,23 +41,23 @@ class AddSecurityVotersPassTest extends TestCase
{
$container = new ContainerBuilder();
$container
->register('security.access.decision_manager', 'Symfony\Component\Security\Core\Authorization\AccessDecisionManager')
->register('security.access.decision_manager', AccessDecisionManager::class)
->addArgument(array())
;
$container
->register('no_prio_service')
->register('no_prio_service', Voter::class)
->addTag('security.voter')
;
$container
->register('lowest_prio_service')
->register('lowest_prio_service', Voter::class)
->addTag('security.voter', array('priority' => 100))
;
$container
->register('highest_prio_service')
->register('highest_prio_service', Voter::class)
->addTag('security.voter', array('priority' => 200))
;
$container
->register('zero_prio_service')
->register('zero_prio_service', Voter::class)
->addTag('security.voter', array('priority' => 0))
;
$compilerPass = new AddSecurityVotersPass();
@ -65,4 +69,56 @@ class AddSecurityVotersPassTest extends TestCase
$this->assertEquals(new Reference('lowest_prio_service'), $refs[1]);
$this->assertCount(4, $refs);
}
/**
* @group legacy
* @expectedDeprecation Using a security.voter tag on a class without implementing the Symfony\Component\Security\Core\Authorization\Voter\VoterInterface is deprecated as of 3.4 and will be removed in 4.0. Implement the Symfony\Component\Security\Core\Authorization\Voter\VoterInterface instead.
*/
public function testVoterMissingInterface()
{
$container = new ContainerBuilder();
$container
->register('security.access.decision_manager', AccessDecisionManager::class)
->addArgument(array())
;
$container
->register('without_interface', VoterWithoutInterface::class)
->addTag('security.voter')
;
$compilerPass = new AddSecurityVotersPass();
$compilerPass->process($container);
$argument = $container->getDefinition('security.access.decision_manager')->getArgument(0);
$refs = $argument->getValues();
$this->assertEquals(new Reference('without_interface'), $refs[0]);
$this->assertCount(1, $refs);
}
/**
* @group legacy
*/
public function testVoterMissingInterfaceAndMethod()
{
$exception = LogicException::class;
$message = 'stdClass should implement the Symfony\Component\Security\Core\Authorization\Voter\VoterInterface interface when used as voter.';
if (method_exists($this, 'expectException')) {
$this->expectException($exception);
$this->expectExceptionMessage($message);
} else {
$this->setExpectedException($exception, $message);
}
$container = new ContainerBuilder();
$container
->register('security.access.decision_manager', AccessDecisionManager::class)
->addArgument(array())
;
$container
->register('without_method', 'stdClass')
->addTag('security.voter')
;
$compilerPass = new AddSecurityVotersPass();
$compilerPass->process($container);
}
}

View File

@ -1,6 +1,12 @@
CHANGELOG
=========
3.4.0
-----
* Using voters that do not implement the `VoterInterface`is now deprecated in
the `AccessDecisionManager` and this functionality will be removed in 4.0.
3.3.0
-----

View File

@ -13,6 +13,7 @@ namespace Symfony\Component\Security\Core\Authorization;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Exception\LogicException;
/**
* AccessDecisionManager is the base class for all access decision managers
@ -84,7 +85,7 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
{
$deny = 0;
foreach ($this->voters as $voter) {
$result = $voter->vote($token, $object, $attributes);
$result = $this->vote($voter, $token, $object, $attributes);
switch ($result) {
case VoterInterface::ACCESS_GRANTED:
return true;
@ -125,7 +126,7 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
$grant = 0;
$deny = 0;
foreach ($this->voters as $voter) {
$result = $voter->vote($token, $object, $attributes);
$result = $this->vote($voter, $token, $object, $attributes);
switch ($result) {
case VoterInterface::ACCESS_GRANTED:
@ -166,7 +167,7 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
$grant = 0;
foreach ($this->voters as $voter) {
foreach ($attributes as $attribute) {
$result = $voter->vote($token, $object, array($attribute));
$result = $this->vote($voter, $token, $object, array($attribute));
switch ($result) {
case VoterInterface::ACCESS_GRANTED:
@ -190,4 +191,27 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
return $this->allowIfAllAbstainDecisions;
}
/**
* TokenInterface vote proxy method.
*
* Acts as a BC layer when the VoterInterface is not implemented on the voter.
*
* @deprecated as of 3.4 and will be removed in 4.0. Call the voter directly as the instance will always be a VoterInterface
*/
private function vote($voter, TokenInterface $token, $subject, $attributes)
{
if ($voter instanceof VoterInterface) {
return $voter->vote($token, $subject, $attributes);
}
if (method_exists($voter, 'vote')) {
@trigger_error(sprintf('Calling vote() on an voter without %1$s is deprecated as of 3.4 and will be removed in 4.0. Implement the %1$s on your voter.', VoterInterface::class), E_USER_DEPRECATED);
// making the assumption that the signature matches
return $voter->vote($token, $subject, $attributes);
}
throw new LogicException(sprintf('%s should implement the %s interface when used as voter.', get_class($voter), VoterInterface::class));
}
}

View File

@ -0,0 +1,21 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\Security\Core\Exception;
/**
* Base LogicException for the Security component.
*
* @author Iltar van der Berg <kjarli@gmail.com>
*/
class LogicException extends \LogicException implements ExceptionInterface
{
}

View File

@ -12,8 +12,11 @@
namespace Symfony\Component\Security\Core\Tests\Authorization;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
use Symfony\Component\Security\Core\Exception\LogicException;
use Symfony\Component\Security\Core\Tests\Authorization\Stub\VoterWithoutInterface;
class AccessDecisionManagerTest extends TestCase
{
@ -138,4 +141,34 @@ class AccessDecisionManagerTest extends TestCase
return $voter;
}
public function testVotingWrongTypeNoVoteMethod()
{
$exception = LogicException::class;
$message = sprintf('stdClass should implement the %s interface when used as voter.', VoterInterface::class);
if (method_exists($this, 'expectException')) {
$this->expectException($exception);
$this->expectExceptionMessage($message);
} else {
$this->setExpectedException($exception, $message);
}
$adm = new AccessDecisionManager(array(new \stdClass()));
$token = $this->getMockBuilder(TokenInterface::class)->getMock();
$adm->decide($token, array('TEST'));
}
/**
* @group legacy
* @expectedDeprecation Calling vote() on an voter without Symfony\Component\Security\Core\Authorization\Voter\VoterInterface is deprecated as of 3.4 and will be removed in 4.0. Implement the Symfony\Component\Security\Core\Authorization\Voter\VoterInterface on your voter.
*/
public function testVotingWrongTypeWithVote()
{
$adm = new AccessDecisionManager(array(new VoterWithoutInterface()));
$token = $this->getMockBuilder(TokenInterface::class)->getMock();
$adm->decide($token, array('TEST'));
}
}

View File

@ -0,0 +1,22 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\Security\Core\Tests\Authorization\Stub;
/**
* @author Iltar van der Berg <kjarli@gmail.com>
*/
class VoterWithoutInterface
{
public function vote()
{
}
}