diff --git a/src/Symfony/Component/Security/Acl/Dbal/MutableAclProvider.php b/src/Symfony/Component/Security/Acl/Dbal/MutableAclProvider.php index 0ac4fa72189e..f24a580aaf52 100644 --- a/src/Symfony/Component/Security/Acl/Dbal/MutableAclProvider.php +++ b/src/Symfony/Component/Security/Acl/Dbal/MutableAclProvider.php @@ -252,6 +252,22 @@ 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) @@ -259,20 +275,20 @@ public function updateAcl(MutableAclInterface $acl) $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 @@ 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(); @@ -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++) { @@ -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; @@ -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]; @@ -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)); } + } diff --git a/src/Symfony/Component/Security/Tests/Acl/Dbal/MutableAclProviderTest.php b/src/Symfony/Component/Security/Tests/Acl/Dbal/MutableAclProviderTest.php index edcdd4d6d5db..00a222801237 100644 --- a/src/Symfony/Component/Security/Tests/Acl/Dbal/MutableAclProviderTest.php +++ b/src/Symfony/Component/Security/Tests/Acl/Dbal/MutableAclProviderTest.php @@ -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(