diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #9485 [Acl] Fix for issue #9433 (guilro)

Browse Source 
This PR was squashed before being merged into the 2.2 branch (closes #9485).

Discussion
----------

[Acl] Fix for issue #9433

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

Two new test for issue #9433 :
`testUpdateAclInsertingMultipleObjectFieldAcesThrowsDBConstraintViolations()`
`testUpdateAclDeletingObjectFieldAcesThrowsDBConstraintViolations()`

The change to `updateAces()` line 857 is enough to make the first test succeed. When changing the `order` field value to a higher value, we must first change the value of the next entry (and all the next entries recursively) to preserve uniqueness of the `order` field in the database.

All the other changes are for the second test. In the former `updateAcl()` method, we commit the changes of the existing ACEs to the database before deleting or adding the new ones. We must delete the old ACEs before changing the existing ACEs in order to preserve uniqueness of the `order` field in the database.

Commits
-------

a38fab9 [Acl] Fix for issue #9433
This commit is contained in:
Fabien Potencier 2013-11-22 18:20:31 +01:00
commit 90dfc9ee08
2 changed files with 148 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

129
src/Symfony/Component/Security/Acl/Dbal/MutableAclProvider.php
View File

@ -252,6 +252,22 @@ class MutableAclProvider extends AclProvider implements MutableAclProviderInterf
}
}
// check properties for deleted, and created ACEs, and perform deletions
// we need to perfom deletions before updating existing ACEs, in order to
// preserve uniqueness of the order field
if (isset($propertyChanges['classAces'])) {
$this->updateOldAceProperty('classAces', $propertyChanges['classAces']);
}
if (isset($propertyChanges['classFieldAces'])) {
$this->updateOldFieldAceProperty('classFieldAces', $propertyChanges['classFieldAces']);
}
if (isset($propertyChanges['objectAces'])) {
$this->updateOldAceProperty('objectAces', $propertyChanges['objectAces']);
}
if (isset($propertyChanges['objectFieldAces'])) {
$this->updateOldFieldAceProperty('objectFieldAces', $propertyChanges['objectFieldAces']);
}
// this includes only updates of existing ACEs, but neither the creation, nor
// the deletion of ACEs; these are tracked by changes to the ACL's respective
// properties (classAces, classFieldAces, objectAces, objectFieldAces)
@ -259,20 +275,20 @@ class MutableAclProvider extends AclProvider implements MutableAclProviderInterf
$this->updateAces($propertyChanges['aces']);
}
// check properties for deleted, and created ACEs
// check properties for deleted, and created ACEs, and perform creations
if (isset($propertyChanges['classAces'])) {
$this->updateAceProperty('classAces', $propertyChanges['classAces']);
$this->updateNewAceProperty('classAces', $propertyChanges['classAces']);
$sharedPropertyChanges['classAces'] = $propertyChanges['classAces'];
}
if (isset($propertyChanges['classFieldAces'])) {
$this->updateFieldAceProperty('classFieldAces', $propertyChanges['classFieldAces']);
$this->updateNewFieldAceProperty('classFieldAces', $propertyChanges['classFieldAces']);
$sharedPropertyChanges['classFieldAces'] = $propertyChanges['classFieldAces'];
}
if (isset($propertyChanges['objectAces'])) {
$this->updateAceProperty('objectAces', $propertyChanges['objectAces']);
$this->updateNewAceProperty('objectAces', $propertyChanges['objectAces']);
}
if (isset($propertyChanges['objectFieldAces'])) {
$this->updateFieldAceProperty('objectFieldAces', $propertyChanges['objectFieldAces']);
$this->updateNewFieldAceProperty('objectFieldAces', $propertyChanges['objectFieldAces']);
}
// if there have been changes to shared properties, we need to synchronize other
@ -740,12 +756,12 @@ QUERY;
}
/**
* This processes changes on an ACE related property (classFieldAces, or objectFieldAces).
* This processes new entries changes on an ACE related property (classFieldAces, or objectFieldAces).
*
* @param string $name
* @param array $changes
*/
private function updateFieldAceProperty($name, array $changes)
private function updateNewFieldAceProperty($name, array $changes)
{
$sids = new \SplObjectStorage();
$classIds = new \SplObjectStorage();
@ -782,6 +798,26 @@ QUERY;
}
}
}
}
/**
* This process old entries changes on an ACE related property (classFieldAces, or objectFieldAces).
*
* @param string $name
* @param array $changes
*/
private function updateOldFieldAceProperty($ane, array $changes)
{
$currentIds = array();
foreach ($changes[1] as $field => $new) {
for ($i=0,$c=count($new); $i<$c; $i++) {
$ace = $new[$i];
if (null != $ace->getId()) {
$currentIds[$ace->getId()] = true;
}
}
}
foreach ($changes[0] as $old) {
for ($i=0,$c=count($old); $i<$c; $i++) {
@ -796,12 +832,12 @@ QUERY;
}
/**
* This processes changes on an ACE related property (classAces, or objectAces).
* This processes new entries changes on an ACE related property (classAces, or objectAces).
*
* @param string $name
* @param array $changes
*/
private function updateAceProperty($name, array $changes)
private function updateNewAceProperty($name, array $changes)
{
list($old, $new) = $changes;
@ -838,6 +874,26 @@ QUERY;
$currentIds[$ace->getId()] = true;
}
}
}
/**
* This processes old entries changes on an ACE related property (classAces, or objectAces).
*
* @param string $name
* @param array $changes
*/
private function updateOldAceProperty($name, array $changes)
{
list($old, $new) = $changes;
$currentIds = array();
for ($i=0,$c=count($new); $i<$c; $i++) {
$ace = $new[$i];
if (null != $ace->getId()) {
$currentIds[$ace->getId()] = true;
}
}
for ($i=0,$c=count($old); $i<$c; $i++) {
$ace = $old[$i];
@ -857,26 +913,41 @@ QUERY;
private function updateAces(\SplObjectStorage $aces)
{
foreach ($aces as $ace) {
$propertyChanges = $aces->offsetGet($ace);
$sets = array();
if (isset($propertyChanges['mask'])) {
$sets[] = sprintf('mask = %d', $propertyChanges['mask'][1]);
}
if (isset($propertyChanges['strategy'])) {
$sets[] = sprintf('granting_strategy = %s', $this->connection->quote($propertyChanges['strategy']));
}
if (isset($propertyChanges['aceOrder'])) {
$sets[] = sprintf('ace_order = %d', $propertyChanges['aceOrder'][1]);
}
if (isset($propertyChanges['auditSuccess'])) {
$sets[] = sprintf('audit_success = %s', $this->connection->getDatabasePlatform()->convertBooleans($propertyChanges['auditSuccess'][1]));
}
if (isset($propertyChanges['auditFailure'])) {
$sets[] = sprintf('audit_failure = %s', $this->connection->getDatabasePlatform()->convertBooleans($propertyChanges['auditFailure'][1]));
}
$this->connection->executeQuery($this->getUpdateAccessControlEntrySql($ace->getId(), $sets));
$this->updateAce($aces, $ace);
}
}
private function updateAce(\SplObjectStorage $aces, $ace)
{
$propertyChanges = $aces->offsetGet($ace);
$sets = array();
if (isset($propertyChanges['aceOrder'])
&& $propertyChanges['aceOrder'][1] > $propertyChanges['aceOrder'][0]
&& $propertyChanges == $aces->offsetGet($ace)) {
$aces->next();
if ($aces->valid()) {
$this->updateAce($aces, $aces->current());
}
}
if (isset($propertyChanges['mask'])) {
$sets[] = sprintf('mask = %d', $propertyChanges['mask'][1]);
}
if (isset($propertyChanges['strategy'])) {
$sets[] = sprintf('granting_strategy = %s', $this->connection->quote($propertyChanges['strategy']));
}
if (isset($propertyChanges['aceOrder'])) {
$sets[] = sprintf('ace_order = %d', $propertyChanges['aceOrder'][1]);
}
if (isset($propertyChanges['auditSuccess'])) {
$sets[] = sprintf('audit_success = %s', $this->connection->getDatabasePlatform()->convertBooleans($propertyChanges['auditSuccess'][1]));
}
if (isset($propertyChanges['auditFailure'])) {
$sets[] = sprintf('audit_failure = %s', $this->connection->getDatabasePlatform()->convertBooleans($propertyChanges['auditFailure'][1]));
}
$this->connection->executeQuery($this->getUpdateAccessControlEntrySql($ace->getId(), $sets));
}
}

48
src/Symfony/Component/Security/Tests/Acl/Dbal/MutableAclProviderTest.php
View File

@ -359,6 +359,54 @@ class MutableAclProviderTest extends \PHPUnit_Framework_TestCase
$this->assertEquals($newParentParentAcl->getId(), $reloadedAcl->getParentAcl()->getParentAcl()->getId());
}
public function testUpdateAclInsertingMultipleObjectFieldAcesThrowsDBConstraintViolations()
{
$provider = $this->getProvider();
$oid = new ObjectIdentity(1, 'Foo');
$sid1 = new UserSecurityIdentity('johannes', 'FooClass');
$sid2 = new UserSecurityIdentity('guilro', 'FooClass');
$sid3 = new UserSecurityIdentity('bmaz', 'FooClass');
$fieldName = 'fieldName';
$acl = $provider->createAcl($oid);
$acl->insertObjectFieldAce($fieldName, $sid1, 4);
$provider->updateAcl($acl);
$acl = $provider->findAcl($oid);
$acl->insertObjectFieldAce($fieldName, $sid2, 4);
$provider->updateAcl($acl);
$acl = $provider->findAcl($oid);
$acl->insertObjectFieldAce($fieldName, $sid3, 4);
$provider->updateAcl($acl);
}
public function testUpdateAclDeletingObjectFieldAcesThrowsDBConstraintViolations()
{
$provider = $this->getProvider();
$oid = new ObjectIdentity(1, 'Foo');
$sid1 = new UserSecurityIdentity('johannes', 'FooClass');
$sid2 = new UserSecurityIdentity('guilro', 'FooClass');
$sid3 = new UserSecurityIdentity('bmaz', 'FooClass');
$fieldName = 'fieldName';
$acl = $provider->createAcl($oid);
$acl->insertObjectFieldAce($fieldName, $sid1, 4);
$provider->updateAcl($acl);
$acl = $provider->findAcl($oid);
$acl->insertObjectFieldAce($fieldName, $sid2, 4);
$provider->updateAcl($acl);
$index = 0;
$acl->deleteObjectFieldAce($index, $fieldName);
$provider->updateAcl($acl);
$acl = $provider->findAcl($oid);
$acl->insertObjectFieldAce($fieldName, $sid3, 4);
$provider->updateAcl($acl);
}
/**
* Data must have the following format:
* array(