Skip to content
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

Jump to bottom

[Acl] Fix for issue #9433 #9485

Closed
wants to merge 2 commits into from
Closed

[Acl] Fix for issue #9433 #9485

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
b60ee4f
[Acl] test for issue #9433
Nov 9, 2013
cefcb16
[Acl] fix bug #9433
Nov 10, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
123 changes: 97 additions & 26 deletions src/Symfony/Component/Security/Acl/Dbal/MutableAclProvider.php
View file
Open in desktop
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
Open in desktop
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