Object ace order issues caused by deleteSecurityIdentity method

[kYem]

Recently I started to look at ACL tables and noticed that all the existing entries are still there even after deleting objects or security identities.

So I looked up and found the two methods that do exactly that. In MutableAclProvider.php
deleteSecurityIdentity() and deleteAcl()

Now deleting ACL is easy and works great when removing a user. However I also want to remove the security identity as well to prevent the issue when someone sign ups with the same user name later and managed to get the same permissions as the previous user.

The problem:
Deleting security identity leaves object ace indexes not in order [0, 1, 3, 4] etc.

This causes issue when trying to modify the ACL for objects that were affected by security identity delete. The method updateOldAceProperty() is called and causes the undefined index notice to be thrown due to the for loop expecting correctly ordered indexes.

MutableAclProvider.php Line 980

for ($i = 0, $c = count($old); $i < $c; ++$i) {
     $ace = $old[$i];
     if (!isset($currentIds[$ace->getId()])) {
        $this->connection->executeQuery($this->getDeleteAccessControlEntrySql($ace->getId()));
        unset($this->loadedAces[$ace->getId()]);
     }
}

Looking at it, all it does is tries to delete the old ace, that was already deleted by by deleteSecurityIdentity() method.

A simple solution would be to transform for loop to foreach, that would fix the index issues

foreach ($old as $ace) {
    if (!isset($currentIds[$ace->getId()])) {
        $this->connection->executeQuery($this->getDeleteAccessControlEntrySql($ace->getId()));
        unset($this->loadedAces[$ace->getId()]);
     }
 }

Let me know if I have wrong approach to this or it seems ok and could be changed in the main repository