8000 [Acl] Fix for issue #9433 · symfony/symfony@a38fab9 · GitHub
[go: up one dir, main page]
Skip to content

Commit a38fab9

Browse files
Guillaume Royerfabpot
Guillaume Royer
authored and
fabpot
committed
[Acl] Fix for issue #9433 
1 parent bde28cb commit a38fab9

File tree

2 files changed

+145
-26
lines changed

2 files changed

+145
-26
lines changed

‎src/Symfony/Component/Security/Acl/Dbal/MutableAclProvider.php

Lines changed: 97 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -252,27 +252,43 @@ public function updateAcl(MutableAclInterface $acl)
252252
}
253253
}
254254

255+
// check properties for deleted, and created ACEs, and perform deletions
256+
// we need to perfom deletions before updating existing ACEs, in order to
257+
// preserve uniqueness of the order field
258+
if (isset($propertyChanges['classAces'])) {
259+
$this->updateOldAceProperty('classAces', $propertyChanges['classAces']);
260+
}
261+
if (isset($propertyChanges['classFieldAces'])) {
262+
$this->updateOldFieldAceProperty('classFieldAces', $propertyChanges['classFieldAces']);
263+
}
264+
if (isset($propertyChanges['objectAces'])) {
265+
$this->updateOldAceProperty('objectAces', $propertyChanges['objectAces']);
266+
}
267+
if (isset($propertyChanges['objectFieldAces'])) {
268+
$this->updateOldFieldAceProperty('objectFieldAces', $propertyChanges['objectFieldAces']);
269+
}
270+
255271
// this includes only updates of existing ACEs, but neither the creation, nor
256272
// the deletion of ACEs; these are tracked by changes to the ACL's respective
257273
// properties (classAces, classFieldAces, objectAces, objectFieldAces)
258274
if (isset($propertyChanges['aces'])) {
259275
$this->updateAces($propertyChanges['aces']);
260276
}
261277

262-
// check properties for deleted, and created ACEs
278+
// check properties for deleted, and created ACEs, and perform creations
263279
if (isset($propertyChanges['classAces'])) {
264-
$this->updateAceProperty('classAces', $propertyChanges['classAces']);
280+
$this->updateNewAceProperty('classAces', $propertyChanges['classAces']);
265281
$sharedPropertyChanges['classAces'] = $propertyChanges['classAces'];
266282
}
267283
if (isset($propertyChanges['classFieldAces'])) {
268-
$this->updateFieldAceProperty('classFieldAces', $propertyChanges['classFieldAces']);
284+
$this->updateNewFieldAceProperty('classFieldAces', $propertyChanges['classFieldAces']);
269285
$sharedPropertyChanges['classFieldAces'] = $propertyChanges['classFieldAces'];
270286
}
271287
if (isset($propertyChanges['objectAces'])) {
272-
$this->updateAceProperty('objectAces', $propertyChanges['objectAces']);
288+
$this->updateNewAceProperty('objectAces', $propertyChanges['objectAces']);
273289
}
274290
if (isset($propertyChanges['objectFieldAces'])) {
275-
$this->updateFieldAceProperty('objectFieldAces', $propertyChanges['objectFieldAces']);
291+
$this->updateNewFieldAceProperty('objectFieldAces', $propertyChanges['objectFieldAces']);
276292
}
277293

278294
// if there have been changes to shared properties, we need to synchronize other
@@ -740,12 +756,12 @@ private function regenerateAncestorRelations(AclInterface $acl)
740756
}
741757

742758
/**
743-
* This processes changes on an ACE related property (classFieldAces, or objectFieldAces).
759+
* This processes new entries changes on an ACE related property (classFieldAces, or objectFieldAces).
744760
*
745761
* @param string $name
746762
* @param array $changes
747763
*/
748-
private function updateFieldAceProperty($name, array $changes)
764+
private function updateNewFieldAceProperty($name, array $changes)
749765
{
750766
$sids = new \SplObjectStorage();
751767
$classIds = new \SplObjectStorage();
@@ -782,6 +798,26 @@ private function updateFieldAceProperty($name, array $changes)
782798
}
783799
}
784800
}
801+
}
802+
803+
/**
804+
* This process old entries changes on an ACE related property (classFieldAces, or objectFieldAces).
805+
*
806+
* @param string $name
807+
* @param array $changes
808+
*/
809+
private function updateOldFieldAceProperty($ane, array $changes)
810+
{
811+
$currentIds = array();
812+
foreach ($changes[1] as $field => $new) {
813+
for ($i=0,$c=count($new); $i<$c; $i++) {
814+
$ace = $new[$i];
815+
816+
if (null != $ace->getId()) {
817+
$currentIds[$ace->getId()] = true;
818+
}
819+
}
820+
}
785821

786822
foreach ($changes[0] as $old) {
787823
for ($i=0,$c=count($old); $i<$c; $i++) {
@@ -796,12 +832,12 @@ private function updateFieldAceProperty($name, array $changes)
796832
}
797833

798834
/**
799-
* This processes changes on an ACE related property (classAces, or objectAces).
835+
* This processes new entries changes on an ACE related property (classAces, or objectAces).
800836
*
801837
* @param string $name
802838
* @param array $changes
803839
*/
804-
private function updateAceProperty($name, array $changes)
840+
private function updateNewAceProperty($name, array $changes)
805841
{
806842
list($old, $new) = $changes;
807843

@@ -838,6 +874,26 @@ private function updateAceProperty($name, array $changes)
838874
$currentIds[$ace->getId()] = true;
839875
}
840876
}
877+
}
878+
879+
/**
880+
* This processes old entries changes on an ACE related property (classAces, or objectAces).
881+
*
882+
* @param string $name
883+
* @param array $changes
884+
*/
885+
private function updateOldAceProperty($name, array $changes)
886+
{
887+
list($old, $new) = $changes;
888+
$currentIds = array();
889+
890+
for ($i=0,$c=count($new); $i<$c; $i++) {
891+
$ace = $new[$i];
892+
893+
if (null != $ace->getId()) {
894+
$currentIds[$ace->getId()] = true;
895+
}
896+
}
841897

842898
for ($i=0,$c=count($old); $i<$c; $i++) {
843899
$ace = $old[$i];
@@ -857,26 +913,41 @@ private function updateAceProperty($name, array $changes)
857913
private function updateAces(\SplObjectStorage $aces)
858914
{
859915
foreach ($aces as $ace) {
860-
$propertyChanges = $aces->offsetGet($ace);
861-
$sets = array();
916+
$this->updateAce($aces, $ace);
917+
}
918+
}
862919

863-
if (isset($propertyChanges['mask'])) {
864-
$sets[] = sprintf('mask = %d', $propertyChanges['mask'][1]);
865-
}
866-
if (isset($propertyChanges['strategy'])) {
867-
$sets[] = sprintf('granting_strategy = %s', $this->connection->quote($propertyChanges['strategy']));
868-
}
869-
if (isset($propertyChanges['aceOrder'])) {
870-
$sets[] = sprintf('ace_order = %d', $propertyChanges['aceOrder'][1]);
871-
}
872-
if (isset($propertyChanges['auditSuccess'])) {
873-
$sets[] = sprintf('audit_success = %s', $this->connection->getDatabasePlatform()->convertBooleans($propertyChanges['auditSuccess'][1]));
874-
}
875-
if (isset($propertyChanges['auditFailure'])) {
876-
$sets[] = sprintf('audit_failure = %s', $this->connection->getDatabasePlatform()->convertBooleans($propertyChanges['auditFailure'][1]));
920+
private function updateAce(\SplObjectStorage $aces, $ace)
921+
{
922+
$propertyChanges = $aces->offsetGet($ace);
923+
$sets = array();
924+
925+
if (isset($propertyChanges['aceOrder'])
926+
&& $propertyChanges['aceOrder'][1] > $propertyChanges['aceOrder'][0]
927+
&& $propertyChanges == $aces->offsetGet($ace)) {
928+
$aces->next();
929+
if ($aces->valid()) {
930+
$this->updateAce($aces, $aces->current());
931+
}
877932
}
878933

879-
$this->connection->executeQuery($this->getUpdateAccessControlEntrySql($ace->getId(), $sets));
934+
if (isset($propertyChanges['mask'])) {
935+
$sets[] = sprintf('mask = %d', $propertyChanges['mask'][1]);
936+
}
937+
if (isset($propertyChanges['strategy'])) {
938+
$sets[] = sprintf('granting_strategy = %s', $this->connection->quote($propertyChanges['strategy']));
939+
}
940+
if (isset($propertyChanges['aceOrder'])) {
941+
$sets[] = sprintf('ace_order = %d', $propertyChanges['aceOrder'][1]);
942+
}
943+
if (isset($propertyChanges['auditSuccess'])) {
944+
$sets[] = sprintf('audit_success = %s', $this->connection->getDatabasePlatform()->convertBooleans($propertyChanges['auditSuccess'][1]));
880945
}
946+
if (isset($propertyChanges['auditFailure'])) {
947+
$sets[] = sprintf('audit_failure = %s', $this->connection->getDatabasePlatform()->convertBooleans($propertyChanges['auditFailure'][1]));
948+
}
949+
950+
$this->connection->executeQuery($this->getUpdateAccessControlEntrySql($ace->getId(), $sets));
881951
}
952+
882953
}

‎src/Symfony/Component/Security/Tests/Acl/Dbal/MutableAclProviderTest.php

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,54 @@ public function testUpdateAclUpdatesChildAclsCorrectly()
359359
$this->assertEquals($newParentParentAcl->getId(), $reloadedAcl->getParentAcl()->getParentAcl()->getId());
360360
}
361361

362+
public function testUpdateAclInsertingMultipleObjectFieldAcesThrowsDBConstraintViolations()
363+
{
364+
$provider = $this->getProvider();
365+
$oid = new ObjectIdentity(1, 'Foo');
366+
$sid1 = new UserSecurityIdentity('johannes', 'FooClass');
367+
$sid2 = new UserSecurityIdentity('guilro', 'FooClass');
368+
$sid3 = new UserSecurityIdentity('bmaz', 'FooClass');
369+
$fieldName = 'fieldName';
370+
371+
$acl = $provider->createAcl($oid);
372+
$acl->insertObjectFieldAce($fieldName, $sid1, 4);
373+
$provider->updateAcl($acl);
374+
375+
$acl = $provider->findAcl($oid);
376+
$acl->insertObjectFieldAce($fieldName, $sid2, 4);
377+
$provider->updateAcl($acl);
378+
379+
$acl = $provider->findAcl($oid);
380+
$acl->insertObjectFieldAce($fieldName, $sid3, 4);
381+
$provider->updateAcl($acl);
382+
}
383+
384+
public function testUpdateAclDeletingObjectFieldAcesThrowsDBConstraintViolations()
385+
{
386+
$provider = $this->getProvider();
387+
$oid = new ObjectIdentity(1, 'Foo');
388+
$sid1 = new UserSecurityIdentity('johannes', 'FooClass');
389+
$sid2 = new UserSecurityIdentity('guilro', 'FooClass');
390+
$sid3 = new UserSecurityIdentity('bmaz', 'FooClass');
391+
$fieldName = 'fieldName';
392+
393+
$acl = $provider->createAcl($oid);
394+
$acl->insertObjectFieldAce($fieldName, $sid1, 4);
395+
$provider->updateAcl($acl);
396+
397+
$acl = $provider->findAcl($oid);
398+
$acl->insertObjectFieldAce($fieldName, $sid2, 4);
399+
$provider->updateAcl($acl);
400+
401+
$index = 0;
402+
$acl->deleteObjectFieldAce($index, $fieldName);
403+
$provider->updateAcl($acl);
404+
405+
$acl = $provider->findAcl($oid);
406+
$acl->insertObjectFieldAce($fieldName, $sid3, 4);
407+
$provider->updateAcl($acl);
408+
}
409+
362410
/**
363411
* Data must have the following format:
364412
* array(

0 commit comments

Comments
 (0)
0