[BUG] Ace order is breaking after the deletion of a security identity

[francoispluchino]

When you delete a security identity with MutableAclProvider::deleteSecurityIdentity(), all ACL entries associated with this security identity are deleted (cascade).

There are 2 problems arising from this behavior:

First, we get an error when we add, modify or delete other ACL entries, if the deleted ACL entries are not the last (the PR #24 partially corrects this problem, but it missing the change to the methods MutableAclProvider::updateNewFieldAceProperty() and MutableAclProvider::updateOldFieldAceProperty().

Secondly, when you add another ACL entry for the same object identifier, sometimes the value of ACE Order is the same as another ACL entry. This happens when you delete a cascading ACL entry that is not the last in order of ACL entries.

Last, it is impossible to reproduce the bug in unit tests with SQLite, because removing cascade not working. The result is that the ACL entry has a security identity with a null value in the database, and that the method findAcls create a RoleSecurityIdentity instance with an empty role name "".

Example:

class MutableAclProviderTest extends \PHPUnit_Framework_TestCase
{
    //...

    public function testDeleteUserSecurityIdentity()
    {
        $provider = $this->getProvider();
        $acl = $provider->createAcl(new ObjectIdentity(1, 'Foo'));
        $sid = new UserSecurityIdentity('johannes', 'FooClass');
        $sid2 = new UserSecurityIdentity('francois', 'FooClass');
        $acl->setEntriesInheriting(!$acl->isEntriesInheriting());

        $acl->insertObjectAce($sid, 1);
        $acl->insertObjectAce($sid2, 2);
        $acl->insertObjectAce($sid, 2);
        $provider->updateAcl($acl);

        $reloadProvider = $this->getProvider();
        $reloadedAcl = $reloadProvider->findAcl(new ObjectIdentity(1, 'Foo'));
        $this->assertCount(3, $reloadedAcl->getObjectAces());

        $reloadProvider->deleteSecurityIdentity($sid2);

        $reloadProviderEmpty = $this->getProvider();
        $reloadedAclEmpty = $reloadProviderEmpty->findAcl(new ObjectIdentity(1, 'Foo'));
        $this->assertNotSame($reloadProvider, $reloadedAclEmpty);
        $this->assertCount(2, $reloadedAclEmpty->getObjectAces());
        // Assert failed because count = 3
    }
}

It's really very disturbing in production.

[francoispluchino]

I close this issue to clean my list and because this library no longer seems to be maintained. But do not hesitate to reopen or merge directly the PR #29 if you wish.

[lisachenko]

SQL procedure to fix ACE ordering https://gist.github.com/lisachenko/6079e77d8d50377033687a85105be0ac. This will re-enumerate ordering of existing ACEs from scratch, but be aware that original ordering is not respected, thus if you app depends on ACE ordering, then do not apply such query.