Skip to content

[Security] Deprecate "AbstractVoter" in favor of "Voter" #16601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 26, 2015
Merged

[Security] Deprecate "AbstractVoter" in favor of "Voter" #16601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d3c6d93
[Security] Revert changes made between 2.7 and 2.8-beta
nicolas-grekas Nov 24, 2015
fd8b87c
[Security] Deprecate "AbstractVoter" in favor of "Voter"
lyrixx Nov 19, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 5 additions & 31 deletions UPGRADE-2.8.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,38 +442,12 @@ FrameworkBundle
Security
--------

* The AbstractToken::isGranted() method was deprecated. Instead,
override the voteOnAttribute() method. This method has one small
difference: it's passed the TokenInterface instead of the user:
* The `AbstractVoter` class was deprecated. Instead, extend the `Voter` class and
move your voting logic in the `supports($attribute, $subject)` and
`voteOnAttribute($attribute, $object, TokenInterface $token)` methods.

Before:

```php
class MyCustomVoter extends AbstractVoter
{
// ...

protected function isGranted($attribute, $object, $user = null)
{
// ...
}
}
```

After:

```php
class MyCustomVoter extends AbstractVoter
{
// ...

protected function voteOnAttribute($attribute, $object, TokenInterface $token)
{
$user = $token->getUser();
// ...
}
}
```
* The `VoterInterface::supportsClass` and `supportsAttribute` methods were
deprecated and will be removed from the interface in 3.0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're deprecating AbstractVoter entirely, then we don't need to talk about it at all, right? The upgrade path would be to switch to Voter and make the changes needed there, correct? If so, I think we should have one simple, before and after (before AbstractVoter after with Voter).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If people were using AbstractVoter in 2.7, they can read how to upgrade to 2.8;
Then they could upgrade with the new Voter. Like that it's done step by step.

But I can merge all steps together.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, we should maybe just revert the changes made on AbstractVoter for 2.8 entirely, but keep the deprecations. Ie remove the supports method & co. Don't you think ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a real BC break

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How that? SF 2.8 is not released, how can it be à BC break to revert the changes made on 2.8?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the implementation sticks on the implementation of the 2.7 version.

Config
------
Expand Down
89 changes: 10 additions & 79 deletions src/Symfony/Component/Security/Core/Authorization/Voter/AbstractVoter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@

namespace Symfony\Component\Security\Core\Authorization\Voter;

@trigger_error('The '.__NAMESPACE__.'\AbstractVoter class is deprecated since version 2.8, to be removed in 3.0. Upgrade to Symfony\Component\Security\Core\Authorization\Voter\Voter instead.', E_USER_DEPRECATED);

use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;

/**
* Abstract Voter implementation that reduces boilerplate code required to create a custom Voter.
*
* @author Roman Marintšenko <inoryy@gmail.com>
*
* @deprecated since version 2.8, to be removed in 3.0. Upgrade to Symfony\Component\Security\Core\Authorization\Voter\Voter instead.
*/
abstract class AbstractVoter implements VoterInterface
{
Expand All @@ -26,8 +30,6 @@ abstract class AbstractVoter implements VoterInterface
*/
public function supportsAttribute($attribute)
{
@trigger_error('The '.__METHOD__.' is deprecated since version 2.8 and will be removed in version 3.0.', E_USER_DEPRECATED);

return in_array($attribute, $this->getSupportedAttributes());
}

Expand All @@ -36,8 +38,6 @@ public function supportsAttribute($attribute)
*/
public function supportsClass($class)
{
@trigger_error('The '.__METHOD__.' is deprecated since version 2.8 and will be removed in version 3.0.', E_USER_DEPRECATED);

foreach ($this->getSupportedClasses() as $supportedClass) {
if ($supportedClass === $class || is_subclass_of($class, $supportedClass)) {
return true;
Expand All @@ -62,22 +62,22 @@ public function supportsClass($class)
*/
public function vote(TokenInterface $token, $object, array $attributes)
{
if (!$object) {
if (!$object || !$this->supportsClass(get_class($object))) {
return self::ACCESS_ABSTAIN;
}

// abstain vote by default in case none of the attributes are supported
$vote = self::ACCESS_ABSTAIN;

foreach ($attributes as $attribute) {
if (!$this->supports($attribute, $object)) {
if (!$this->supportsAttribute($attribute)) {
continue;
}

// as soon as at least one attribute is supported, default is to deny access
$vote = self::ACCESS_DENIED;

if ($this->voteOnAttribute($attribute, $object, $token)) {
if ($this->isGranted($attribute, $object, $token->getUser())) {
// grant access as soon as at least one voter returns a positive response
return self::ACCESS_GRANTED;
}
Expand All @@ -86,62 +86,19 @@ public function vote(TokenInterface $token, $object, array $attributes)
return $vote;
}

/**
* Determines if the attribute and object are supported by this voter.
*
* This method will become abstract in 3.0.
*
* @param string $attribute An attribute
* @param string $object The object to secure
*
* @return bool True if the attribute and object is supported, false otherwise
*/
protected function supports($attribute, $object)
{
@trigger_error('The getSupportedClasses and getSupportedAttributes methods are deprecated since version 2.8 and will be removed in version 3.0. Overwrite supports instead.', E_USER_DEPRECATED);

$classIsSupported = false;
foreach ($this->getSupportedClasses() as $supportedClass) {
if ($object instanceof $supportedClass) {
$classIsSupported = true;
break;
}
}

if (!$classIsSupported) {
return false;
}

if (!in_array($attribute, $this->getSupportedAttributes())) {
return false;
}

return true;
}

/**
* Return an array of supported classes. This will be called by supportsClass.
*
* @return array an array of supported classes, i.e. array('Acme\DemoBundle\Model\Product')
*
* @deprecated since version 2.8, to be removed in 3.0. Use supports() instead.
*/
protected function getSupportedClasses()
{
@trigger_error('The '.__METHOD__.' is deprecated since version 2.8 and will be removed in version 3.0.', E_USER_DEPRECATED);
}
abstract protected function getSupportedClasses();

/**
* Return an array of supported attributes. This will be called by supportsAttribute.
*
* @return array an array of supported attributes, i.e. array('CREATE', 'READ')
*
* @deprecated since version 2.8, to be removed in 3.0. Use supports() instead.
*/
protected function getSupportedAttributes()
{
@trigger_error('The '.__METHOD__.' is deprecated since version 2.8 and will be removed in version 3.0.', E_USER_DEPRECATED);
}
abstract protected function getSupportedAttributes();

/**
* Perform a single access check operation on a given attribute, object and (optionally) user
Expand All @@ -154,33 +111,7 @@ protected function getSupportedAttributes()
* @param object $object
* @param UserInterface|string $user
*
* @deprecated This method will be removed in 3.0 - override voteOnAttribute instead.
*
* @return bool
*/
protected function isGranted($attribute, $object, $user = null)
{
// forces isGranted() or voteOnAttribute() to be overridden
throw new \BadMethodCallException(sprintf('You must override the voteOnAttribute() method in "%s".', get_class($this)));
}

/**
* Perform a single access check operation on a given attribute, object and token.
* It is safe to assume that $attribute and $object's class pass supports method call.
*
* This method will become abstract in 3.0.
*
* @param string $attribute
* @param object $object
* @param TokenInterface $token
*
* @return bool
*/
protected function voteOnAttribute($attribute, $object, TokenInterface $token)
{
// the user should override this method, and not rely on the deprecated isGranted()
@trigger_error(sprintf("The AbstractVoter::isGranted() method is deprecated since 2.8 and won't be called anymore in 3.0. Override voteOnAttribute() in %s instead.", get_class($this)), E_USER_DEPRECATED);

return $this->isGranted($attribute, $object, $token->getUser());
}
abstract protected function isGranted($attribute, $object, $user = null);
}
85 changes: 85 additions & 0 deletions src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<?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\Authorization\Voter;

use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;

/**
* Voter is an abstract default implementation of a voter.
*
* @author Roman Marintšenko <inoryy@gmail.com>
* @author Grégoire Pineau <lyrixx@lyrixx.info>
*/
abstract class Voter implements VoterInterface
{
/**
* {@inheritdoc}
*/
public function supportsAttribute($attribute)
{
throw new \BadMethodCallException('supportsAttribute method is deprecated since version 2.8, to be removed in 3.0');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the AbstractVoter still works in 2.8 (but is deprecated of course), do we need these methods at all in the new class? When I switch to the new class, I will change to use the new methods.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, to implements the interface :/


/**
* {@inheritdoc}
*/
public function supportsClass($class)
{
throw new \BadMethodCallException('supportsClass method is deprecated since version 2.8, to be removed in 3.0');
}

/**
* {@inheritdoc}
*/
public function vote(TokenInterface $token, $object, array $attributes)
{
// abstain vote by default in case none of the attributes are supported
$vote = self::ACCESS_ABSTAIN;

foreach ($attributes as $attribute) {
if (!$this->supports($attribute, $object)) {
continue;
}

// as soon as at least one attribute is supported, default is to deny access
$vote = self::ACCESS_DENIED;

if ($this->voteOnAttribute($attribute, $object, $token)) {
// grant access as soon as at least one attribute returns a positive response
return self::ACCESS_GRANTED;
}
}

return $vote;
}

/**
* Determines if the attribute and subject are supported by this voter.
*
* @param string $attribute An attribute
* @param mixed $subject The subject to secure, e.g. an object the user wants to access or any other PHP type
*
* @return bool True if the attribute and subject are supported, false otherwise
*/
abstract protected function supports($attribute, $subject);

/**
* Perform a single access check operation on a given attribute, subject and token.
*
* @param string $attribute
* @param mixed $subject
* @param TokenInterface $token
*
* @return bool
*/
abstract protected function voteOnAttribute($attribute, $subject, TokenInterface $token);
}
74 changes: 4 additions & 70 deletions src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AbstractVoterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@

namespace Symfony\Component\Security\Core\Tests\Authorization\Voter;

use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authorization\Voter\AbstractVoter;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;

/**
* @group legacy
*/
class AbstractVoterTest extends \PHPUnit_Framework_TestCase
{
protected $token;
Expand Down Expand Up @@ -50,75 +51,8 @@ public function getTests()
*/
public function testVote(array $attributes, $expectedVote, $object, $message)
{
$voter = new AbstractVoterTest_Voter();
$voter = new Fixtures\MyVoter();

$this->assertEquals($expectedVote, $voter->vote($this->token, $object, $attributes), $message);
}

/**
* @dataProvider getTests
* @group legacy
*/
public function testVoteLegacy(array $attributes, $expectedVote, $object, $message)
{
$voter = new AbstractVoterTest_LegacyVoter();

$this->assertEquals($expectedVote, $voter->vote($this->token, $object, $attributes), $message);
}

/**
* @group legacy
* @expectedException \BadMethodCallException
*/
public function testNoOverriddenMethodsThrowsException()
{
$voter = new AbstractVoterTest_NothingImplementedVoter();
$voter->vote($this->token, new \stdClass(), array('EDIT'));
}
}

class AbstractVoterTest_Voter extends AbstractVoter
{
protected function voteOnAttribute($attribute, $object, TokenInterface $token)
{
return 'EDIT' === $attribute;
}

protected function supports($attribute, $object)
{
return $object instanceof \stdClass && in_array($attribute, array('EDIT', 'CREATE'));
}
}

class AbstractVoterTest_LegacyVoter extends AbstractVoter
{
protected function getSupportedClasses()
{
return array('stdClass');
}

protected function getSupportedAttributes()
{
return array('EDIT', 'CREATE');
}

protected function isGranted($attribute, $object, $user = null)
{
return 'EDIT' === $attribute;
}
}

class AbstractVoterTest_NothingImplementedVoter extends AbstractVoter
{
protected function getSupportedClasses()
{
return array('stdClass');
}

protected function getSupportedAttributes()
{
return array('EDIT', 'CREATE');
}

// this is a bad voter that hasn't overridden isGranted or voteOnAttribute
}
Loading