Skip to content

Commit

Permalink
[Acl] Fix for issue #9433
Browse files Browse the repository at this point in the history
  • Loading branch information
@fabpot
Guillaume Royer authored and fabpot committed Nov 22, 2013
1 parent bde28cb commit a38fab9
Show file tree
Hide file tree
Showing 2 changed files with 145 additions and 26 deletions.
123 changes: 97 additions & 26 deletions src/Symfony/Component/Security/Acl/Dbal/MutableAclProvider.php
View file
Expand Up @@ -252,27 +252,43 @@ public function updateAcl(MutableAclInterface $acl)
}
}

// 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)
if (isset($propertyChanges['aces'])) {
$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
Expand Down Expand Up @@ -740,12 +756,12 @@ private function regenerateAncestorRelations(AclInterface $acl)
}

/**
* 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();
Expand Down Expand Up @@ -782,6 +798,26 @@ private function updateFieldAceProperty($name, array $changes)
}
}
}
}

/**
* 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++) {
Expand All @@ -796,12 +832,12 @@ private function updateFieldAceProperty($name, array $changes)
}

/**
* 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;

Expand Down Expand Up @@ -838,6 +874,26 @@ private function updateAceProperty($name, array $changes)
$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];
Expand All @@ -857,26 +913,41 @@ private function updateAceProperty($name, array $changes)
private function updateAces(\SplObjectStorage $aces)
{
foreach ($aces as $ace) {
$propertyChanges = $aces->offsetGet($ace);
$sets = array();
$this->updateAce($aces, $ace);
}
}

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]));
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());
}
}

$this->connection->executeQuery($this->getUpdateAccessControlEntrySql($ace->getId(), $sets));
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 changes: 48 additions & 0 deletions src/Symfony/Component/Security/Tests/Acl/Dbal/MutableAclProviderTest.php
View file
Expand Up @@ -359,6 +359,54 @@ public function testUpdateAclUpdatesChildAclsCorrectly()
$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(
Expand Down

0 comments on commit a38fab9

Please sign in to comment.