bug #10863 [Security] Add check for supported attributes in AclVoter (artursvonda)

This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #10863).

Discussion
----------

[Security] Add check for supported attributes in AclVoter

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Couldn't get the tests to pass but the issues where unrelated to change.

This fixes issues with Attribute containing ExpressionLanguage instance when using allow_if.

Commits
-------

a605a3d [Security] Add check for supported attributes in AclVoter
This commit is contained in:
Fabien Potencier 2014-05-08 18:38:49 +02:00
commit d51c9b3bda
2 changed files with 34 additions and 3 deletions

View File

@ -48,12 +48,16 @@ class AclVoter implements VoterInterface
public function supportsAttribute($attribute)
{
return $this->permissionMap->contains($attribute);
return is_string($attribute) && $this->permissionMap->contains($attribute);
}
public function vote(TokenInterface $token, $object, array $attributes)
{
foreach ($attributes as $attribute) {
if (!$this->supportsAttribute($attribute)) {
continue;
}
if (null === $masks = $this->permissionMap->getMasks($attribute, $object)) {
continue;
}

View File

@ -27,7 +27,7 @@ class AclVoterTest extends \PHPUnit_Framework_TestCase
*/
public function testSupportsAttribute($attribute, $supported)
{
list($voter,, $permissionMap,,) = $this->getVoter();
list($voter,, $permissionMap,,) = $this->getVoter(true, false);
$permissionMap
->expects($this->once())
@ -39,6 +39,16 @@ class AclVoterTest extends \PHPUnit_Framework_TestCase
$this->assertSame($supported, $voter->supportsAttribute($attribute));
}
/**
* @dataProvider getSupportsAttributeNonStringTests
*/
public function testSupportsAttributeNonString($attribute)
{
list($voter,,,,,) = $this->getVoter(true, false);
$this->assertFalse($voter->supportsAttribute($attribute));
}
public function getSupportsAttributeTests()
{
return array(
@ -47,6 +57,16 @@ class AclVoterTest extends \PHPUnit_Framework_TestCase
);
}
public function getSupportsAttributeNonStringTests()
{
return array(
array(new \stdClass()),
array(1),
array(true),
array(array()),
);
}
/**
* @dataProvider getSupportsClassTests
*/
@ -387,13 +407,20 @@ class AclVoterTest extends \PHPUnit_Framework_TestCase
return $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
}
protected function getVoter($allowIfObjectIdentityUnavailable = true)
protected function getVoter($allowIfObjectIdentityUnavailable = true, $alwaysContains = true)
{
$provider = $this->getMock('Symfony\Component\Security\Acl\Model\AclProviderInterface');
$permissionMap = $this->getMock('Symfony\Component\Security\Acl\Permission\PermissionMapInterface');
$oidStrategy = $this->getMock('Symfony\Component\Security\Acl\Model\ObjectIdentityRetrievalStrategyInterface');
$sidStrategy = $this->getMock('Symfony\Component\Security\Acl\Model\SecurityIdentityRetrievalStrategyInterface');
if ($alwaysContains) {
$permissionMap
->expects($this->any())
->method('contains')
->will($this->returnValue(true));
}
return array(
new AclVoter($provider, $oidStrategy, $sidStrategy, $permissionMap, null, $allowIfObjectIdentityUnavailable),
$provider,