optimized AclVoter, added unit test

2 files changed, 66 insertions, 23 deletions

diff --git a/Acl/Domain/ObjectIdentity.php b/Acl/Domain/ObjectIdentity.php
index 0de7fdd..57a17f8 100644
--- a/Acl/Domain/ObjectIdentity.php
+++ b/Acl/Domain/ObjectIdentity.php
@@ -58,10 +58,14 @@ class ObjectIdentity implements ObjectIdentityInterface
             throw new InvalidDomainObjectException('$domainObject must be an object.');
         }
 
-        if ($domainObject instanceof DomainObjectInterface) {
-            return new self($domainObject->getObjectIdentifier(), get_class($domainObject));
-        } else if (method_exists($domainObject, 'getId')) {
-            return new self($domainObject->getId(), get_class($domainObject));
+        try {
+            if ($domainObject instanceof DomainObjectInterface) {
+                return new self($domainObject->getObjectIdentifier(), get_class($domainObject));
+            } else if (method_exists($domainObject, 'getId')) {
+                return new self($domainObject->getId(), get_class($domainObject));
+            }
+        } catch (\InvalidArgumentException $invalid) {
+            throw new InvalidDomainObjectException($invalid->getMessage(), 0, $invalid);
         }
 
         throw new InvalidDomainObjectException('$domainObject must either implement the DomainObjectInterface, or have a method named "getId".');
diff --git a/Acl/Voter/AclVoter.php b/Acl/Voter/AclVoter.php
index 954ad9b..002e891 100644
--- a/Acl/Voter/AclVoter.php
+++ b/Acl/Voter/AclVoter.php
@@ -2,6 +2,7 @@
 
 namespace Symfony\Component\Security\Acl\Voter;
 
+use Symfony\Component\HttpKernel\Log\LoggerInterface;
 use Symfony\Component\Security\Acl\Domain\ObjectIdentity;
 use Symfony\Component\Security\Acl\Domain\RoleSecurityIdentity;
 use Symfony\Component\Security\Acl\Domain\UserSecurityIdentity;
@@ -35,13 +36,17 @@ class AclVoter implements VoterInterface
     protected $permissionMap;
     protected $objectIdentityRetrievalStrategy;
     protected $securityIdentityRetrievalStrategy;
+    protected $allowIfObjectIdentityUnavailable;
+    protected $logger;
 
-    public function __construct(AclProviderInterface $aclProvider, ObjectIdentityRetrievalStrategyInterface $oidRetrievalStrategy, SecurityIdentityRetrievalStrategyInterface $sidRetrievalStrategy, PermissionMapInterface $permissionMap)
+    public function __construct(AclProviderInterface $aclProvider, ObjectIdentityRetrievalStrategyInterface $oidRetrievalStrategy, SecurityIdentityRetrievalStrategyInterface $sidRetrievalStrategy, PermissionMapInterface $permissionMap, LoggerInterface $logger = null, $allowIfObjectIdentityUnavailable = true)
     {
         $this->aclProvider = $aclProvider;
         $this->permissionMap = $permissionMap;
         $this->objectIdentityRetrievalStrategy = $oidRetrievalStrategy;
         $this->securityIdentityRetrievalStrategy = $sidRetrievalStrategy;
+        $this->logger = $logger;
+        $this->allowIfObjectIdentityUnavailable = $allowIfObjectIdentityUnavailable;
     }
 
     public function supportsAttribute($attribute)
@@ -51,44 +56,78 @@ class AclVoter implements VoterInterface
 
     public function vote(TokenInterface $token, $object, array $attributes)
     {
-        if (null === $object) {
-            return self::ACCESS_ABSTAIN;
-        } else if ($object instanceof FieldVote) {
-            $field = $object->getField();
-            $object = $object->getDomainObject();
-        } else {
-            $field = null;
-        }
-
-        if (null === $oid = $this->objectIdentityRetrievalStrategy->getObjectIdentity($object)) {
-            return self::ACCESS_ABSTAIN;
-        }
-        $sids = $this->securityIdentityRetrievalStrategy->getSecurityIdentities($token);
-
+        $firstCall = true;
         foreach ($attributes as $attribute) {
             if (!$this->supportsAttribute($attribute)) {
                 continue;
             }
 
-            try {
-                $acl = $this->aclProvider->findAcl($oid, $sids);
-            } catch (AclNotFoundException $noAcl) {
-                return self::ACCESS_DENIED;
+            if ($firstCall) {
+                $firstCall = false;
+
+                if (null === $object) {
+                    if (null !== $this->logger) {
+                        $this->logger->debug(sprintf('Object identity unavailable. Voting to %s', $this->allowIfObjectIdentityUnavailable? 'grant access' : 'abstain'));
+                    }
+
+                    return $this->allowIfObjectIdentityUnavailable ? self::ACCESS_GRANTED : self::ACCESS_ABSTAIN;
+                } else if ($object instanceof FieldVote) {
+                    $field = $object->getField();
+                    $object = $object->getDomainObject();
+                } else {
+                    $field = null;
+                }
+
+                if (null === $oid = $this->objectIdentityRetrievalStrategy->getObjectIdentity($object)) {
+                    if (null !== $this->logger) {
+                        $this->logger->debug(sprintf('Object identity unavailable. Voting to %s', $this->allowIfObjectIdentityUnavailable? 'grant access' : 'abstain'));
+                    }
+
+                    return $this->allowIfObjectIdentityUnavailable ? self::ACCESS_GRANTED : self::ACCESS_ABSTAIN;
+                }
+                $sids = $this->securityIdentityRetrievalStrategy->getSecurityIdentities($token);
+
+                try {
+                    $acl = $this->aclProvider->findAcl($oid, $sids);
+                } catch (AclNotFoundException $noAcl) {
+                    if (null !== $this->logger) {
+                        $this->logger->debug('No ACL found for the object identity. Voting to deny access.');
+                    }
+
+                    return self::ACCESS_DENIED;
+                }
             }
 
             try {
                 if (null === $field && $acl->isGranted($this->permissionMap->getMasks($attribute), $sids, false)) {
+                    if (null !== $this->logger) {
+                        $this->logger->debug('ACL found, permission granted. Voting to grant access');
+                    }
+
                     return self::ACCESS_GRANTED;
                 } else if (null !== $field && $acl->isFieldGranted($field, $this->permissionMap->getMasks($attribute), $sids, false)) {
+                    if (null !== $this->logger) {
+                        $this->logger->debug('ACL found, permission granted. Voting to grant access');
+                    }
+
                     return self::ACCESS_GRANTED;
                 } else {
+                    if (null !== $this->logger) {
+                        $this->logger->debug('ACL found, insufficient permissions. Voting to deny access.');
+                    }
+
                     return self::ACCESS_DENIED;
                 }
             } catch (NoAceFoundException $noAce) {
+                if (null !== $this->logger) {
+                    $this->logger->debug('ACL found, no ACE applicable. Voting to deny access.');
+                }
+
                 return self::ACCESS_DENIED;
             }
         }
 
+        // no attribute was supported
         return self::ACCESS_ABSTAIN;
     }