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.

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

Mutable ACL Provider #23

Open
rtoledof opened this issue Jun 10, 2016 · 2 comments
Open

Mutable ACL Provider #23

rtoledof opened this issue Jun 10, 2016 · 2 comments

Comments

@rtoledof
Copy link

rtoledof commented Jun 10, 2016

Hello guys, I'm using symfony 2.8 and ACL in order to check the permissions for all users in every action in my application. I have a problem with Mutable ACL provider. My problem is in some please I'm trying to remove the user permissions and I'm using the method deleteClassAce in the ACL using the respective index for the specific user after executed the method and try the update ACL the ACL response "Notice: Undefined offset: 4". Looking in the ACL class I saw in the deleteAce method you are executing an unset in the aces arrey "unset($aces[$index]);" and this deleted the element in the array but not reset the array positions and the index in the array are lost for example.

Before executed the deleteClassAce I have these index (0,1,2,3,4), after execute deleteClassAce using index 2 the new index are (0,1,3,4) instead of (0,1,2,3) it's normal because it is a typical behavior in the php arrays the problem is, after executed the deleteClassAce I need to give other permissions in the ACL I'm using and in MutableAclProvider you have this code in updateOldAceProperty method.

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];

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

As you can see you are iterating the array using index and I have in my $old ACL the index (0,1,3,4). the for loop create incremental index and the index 2 is generated after increase variable $i in the array and in the $ace asignation of cource if $i = 2 the index $old[$i] not exist at this point and where it where the notice is throw.

In my local server I tested the foreach loop instead of for loop and the ACL works fine this is the code I change in the method.

The original method looks like this.

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];

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

and my method looks like this

private function updateOldAceProperty($name, array $changes)
    {
        list($old, $new) = $changes;
        $currentIds = array();
        foreach($new as $key => $ace){
         if (null !== $ace->getId()) {
        $currentIds[$ace->getId()] = true;
        }
    }

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

and it works for me.

Is it the solution or exist other problem for user foreach loop instead of for loop?

@anis-marrouchi
Copy link

anis-marrouchi commented Oct 12, 2016

I have faced the same problem, I'm using also Symfony 2.8. To overcome the problem I've used a foreach loop like you have suggested. The same problem occured with the updateNewAceProperty method of MutableAclProvider . I've used also the foreach loop. If anyone is facing the same problem use your own MutableAclProvider and overwrite the 2 methods.

private function updateNewAceProperty($name, array $changes) {
        list($old, $new) = $changes;

        $sids = new SplObjectStorage();
        $classIds = new SplObjectStorage();
        foreach ($new as $i => $ace) {
            if (null === $ace->getId()) {
                if ($sids->contains($ace->getSecurityIdentity())) {
                    $sid = $sids->offsetGet($ace->getSecurityIdentity());
                } else {
                    $sid = $this->createOrRetrieveSecurityIdentityId($ace->getSecurityIdentity());
                }

                $oid = $ace->getAcl()->getObjectIdentity();
                if ($classIds->contains($oid)) {
                    $classId = $classIds->offsetGet($oid);
                } else {
                    $classId = $this->createOrRetrieveClassId($oid->getType());
                }

                $objectIdentityId = $name === 'classAces' ? null : $ace->getAcl()->getId();

                $this->connection->executeQuery($this->getInsertAccessControlEntrySql($classId, $objectIdentityId, null, $i, $sid, $ace->getStrategy(), $ace->getMask(), $ace->isGranting(), $ace->isAuditSuccess(), $ace->isAuditFailure()));
                $aceId = $this->connection->executeQuery($this->getSelectAccessControlEntryIdSql($classId, $objectIdentityId, null, $i))->fetchColumn();
                $this->loadedAces[$aceId] = $ace;

                $aceIdProperty = new ReflectionProperty($ace, 'id');
                $aceIdProperty->setAccessible(true);
                $aceIdProperty->setValue($ace, (int) $aceId);
            }
        }
    }

@francoispluchino
Copy link

@rtoledof The PR #29 fixes this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants