Merge branch '5.4' into 6.0

xabbuh · xabbuh · commit 1cc7909dc7ef · 2021-10-29T09:35:21.000+02:00
* 5.4: (33 commits)
  do not pass a boolean to the constructor of the Dotenv class
  [Notifier] Fix low-deps tests
  Deprecate Composer 1
  [Filesystem] Add third argument `$lockFile` to `Filesystem::appendToFile()`
  [TwigBundle] fix auto-enabling assets/expression/routing/yaml/workflow extensions
  [PhpUnitBridge] fix symlink to bridge in docker by making its path relative
  [Dotenv] Duplicate $_SERVER values in $_ENV if they don't exist
  Fix markup
  [Notifier] Add Expo bridge
  Add polish translations (#43725)
  Rename translation:update to translation:extract
  [String] Add PLURAL_MAP "zombies" -- fix #43789
  Added support for `statusCode` default parameter when loading a template directly from route using the `Symfony\Bundle\FrameworkBundle\Controller\TemplateController` controller.
  [Validator] Update validators.bs.xlf
  Cache voters that will always abstain
  [Messenger] Fix `TraceableMessageBus` implementation so it can compute caller even when used within a callback
  [Validator] Add the missing translations for Russian (ru)
  [Validator] Added missing translations for Latvian (lv)
  [Validator] Added missing translations
  [Validator] Add missing translations for Vietnamese (VI)
  ...
diff --git a/Authorization/AccessDecisionManager.php b/Authorization/AccessDecisionManager.php
@@ -12,6 +12,7 @@
 namespace Symfony\Component\Security\Core\Authorization;
 
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface;
 use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
 use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
 
@@ -29,6 +30,8 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
     public const STRATEGY_PRIORITY = 'priority';
 
     private $voters;
+    private $votersCacheAttributes;
+    private $votersCacheObject;
     private $strategy;
     private $allowIfAllAbstainDecisions;
     private $allowIfEqualGrantedDeniedDecisions;
@@ -78,7 +81,7 @@ public function decide(TokenInterface $token, array $attributes, mixed $object =
     private function decideAffirmative(TokenInterface $token, array $attributes, mixed $object = null): bool
     {
         $deny = 0;
-        foreach ($this->voters as $voter) {
+        foreach ($this->getVoters($attributes, $object) as $voter) {
             $result = $voter->vote($token, $object, $attributes);
 
             if (VoterInterface::ACCESS_GRANTED === $result) {
@@ -117,7 +120,7 @@ private function decideConsensus(TokenInterface $token, array $attributes, mixed
     {
         $grant = 0;
         $deny = 0;
-        foreach ($this->voters as $voter) {
+        foreach ($this->getVoters($attributes, $object) as $voter) {
             $result = $voter->vote($token, $object, $attributes);
 
             if (VoterInterface::ACCESS_GRANTED === $result) {
@@ -153,7 +156,7 @@ private function decideConsensus(TokenInterface $token, array $attributes, mixed
     private function decideUnanimous(TokenInterface $token, array $attributes, mixed $object = null): bool
     {
         $grant = 0;
-        foreach ($this->voters as $voter) {
+        foreach ($this->getVoters($attributes, $object) as $voter) {
             foreach ($attributes as $attribute) {
                 $result = $voter->vote($token, $object, [$attribute]);
 
@@ -186,7 +189,7 @@ private function decideUnanimous(TokenInterface $token, array $attributes, mixed
      */
     private function decidePriority(TokenInterface $token, array $attributes, mixed $object = null)
     {
-        foreach ($this->voters as $voter) {
+        foreach ($this->getVoters($attributes, $object) as $voter) {
             $result = $voter->vote($token, $object, $attributes);
 
             if (VoterInterface::ACCESS_GRANTED === $result) {
@@ -204,4 +207,48 @@ private function decidePriority(TokenInterface $token, array $attributes, mixed
 
         return $this->allowIfAllAbstainDecisions;
     }
+
+    private function getVoters(array $attributes, $object = null): iterable
+    {
+        $keyAttributes = [];
+        foreach ($attributes as $attribute) {
+            $keyAttributes[] = \is_string($attribute) ? $attribute : null;
+        }
+        // use `get_class` to handle anonymous classes
+        $keyObject = \is_object($object) ? \get_class($object) : get_debug_type($object);
+        foreach ($this->voters as $key => $voter) {
+            if (!$voter instanceof CacheableVoterInterface) {
+                yield $voter;
+                continue;
+            }
+
+            $supports = true;
+            // The voter supports the attributes if it supports at least one attribute of the list
+            foreach ($keyAttributes as $keyAttribute) {
+                if (null === $keyAttribute) {
+                    $supports = true;
+                } elseif (!isset($this->votersCacheAttributes[$keyAttribute][$key])) {
+                    $this->votersCacheAttributes[$keyAttribute][$key] = $supports = $voter->supportsAttribute($keyAttribute);
+                } else {
+                    $supports = $this->votersCacheAttributes[$keyAttribute][$key];
+                }
+                if ($supports) {
+                    break;
+                }
+            }
+            if (!$supports) {
+                continue;
+            }
+
+            if (!isset($this->votersCacheObject[$keyObject][$key])) {
+                $this->votersCacheObject[$keyObject][$key] = $supports = $voter->supportsType($keyObject);
+            } else {
+                $supports = $this->votersCacheObject[$keyObject][$key];
+            }
+            if (!$supports) {
+                continue;
+            }
+            yield $voter;
+        }
+    }
 }
diff --git a/Authorization/Voter/AuthenticatedVoter.php b/Authorization/Voter/AuthenticatedVoter.php
@@ -24,7 +24,7 @@
  * @author Fabien Potencier <fabien@symfony.com>
  * @author Johannes M. Schmitt <schmittjoh@gmail.com>
  */
-class AuthenticatedVoter implements VoterInterface
+class AuthenticatedVoter implements CacheableVoterInterface
 {
     public const IS_AUTHENTICATED_FULLY = 'IS_AUTHENTICATED_FULLY';
     public const IS_AUTHENTICATED_REMEMBERED = 'IS_AUTHENTICATED_REMEMBERED';
@@ -87,4 +87,23 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes):
 
         return $result;
     }
+
+    public function supportsAttribute(string $attribute): bool
+    {
+        return \in_array($attribute, [
+            self::IS_AUTHENTICATED_FULLY,
+            self::IS_AUTHENTICATED_REMEMBERED,
+            self::IS_AUTHENTICATED_ANONYMOUSLY,
+            self::IS_AUTHENTICATED,
+            self::IS_ANONYMOUS,
+            self::IS_IMPERSONATOR,
+            self::IS_REMEMBERED,
+            self::PUBLIC_ACCESS,
+        ], true);
+    }
+
+    public function supportsType(string $subjectType): bool
+    {
+        return true;
+    }
 }
diff --git a/Authorization/Voter/CacheableVoterInterface.php b/Authorization/Voter/CacheableVoterInterface.php
@@ -0,0 +1,30 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Core\Authorization\Voter;
+
+/**
+ * Let voters expose the attributes and types they care about.
+ *
+ * By returning false to either `supportsAttribute` or `supportsType`, the
+ * voter will never be called for the specified attribute or subject.
+ *
+ * @author Jérémy Derussé <jeremy@derusse.com>
+ */
+interface CacheableVoterInterface extends VoterInterface
+{
+    public function supportsAttribute(string $attribute): bool;
+
+    /**
+     * @param string $subjectType The type of the subject inferred by `get_class` or `get_debug_type`
+     */
+    public function supportsType(string $subjectType): bool;
+}
diff --git a/Authorization/Voter/RoleVoter.php b/Authorization/Voter/RoleVoter.php
@@ -18,7 +18,7 @@
  *
  * @author Fabien Potencier <fabien@symfony.com>
  */
-class RoleVoter implements VoterInterface
+class RoleVoter implements CacheableVoterInterface
 {
     private $prefix;
 
@@ -51,6 +51,16 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes):
         return $result;
     }
 
+    public function supportsAttribute(string $attribute): bool
+    {
+        return str_starts_with($attribute, $this->prefix);
+    }
+
+    public function supportsType(string $subjectType): bool
+    {
+        return true;
+    }
+
     protected function extractRoles(TokenInterface $token)
     {
         return $token->getRoleNames();
diff --git a/Authorization/Voter/TraceableVoter.php b/Authorization/Voter/TraceableVoter.php
@@ -22,7 +22,7 @@
  *
  * @internal
  */
-class TraceableVoter implements VoterInterface
+class TraceableVoter implements CacheableVoterInterface
 {
     private $voter;
     private $eventDispatcher;
@@ -46,4 +46,14 @@ public function getDecoratedVoter(): VoterInterface
     {
         return $this->voter;
     }
+
+    public function supportsAttribute(string $attribute): bool
+    {
+        return !$this->voter instanceof CacheableVoterInterface || $this->voter->supportsAttribute($attribute);
+    }
+
+    public function supportsType(string $subjectType): bool
+    {
+        return !$this->voter instanceof CacheableVoterInterface || $this->voter->supportsType($subjectType);
+    }
 }
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@ CHANGELOG
 5.4
 ---
 
+ * Add a `CacheableVoterInterface` for voters that vote only on identified attributes and subjects
  * Deprecate `AuthenticationEvents::AUTHENTICATION_FAILURE`, use the `LoginFailureEvent` instead
  * Deprecate `AnonymousToken`, as the related authenticator was deprecated in 5.3
  * Deprecate `Token::getCredentials()`, tokens should no longer contain credentials (as they represent authenticated sessions)
diff --git a/Tests/Authorization/AccessDecisionManagerTest.php b/Tests/Authorization/AccessDecisionManagerTest.php
@@ -14,6 +14,7 @@
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
+use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface;
 use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
 
 class AccessDecisionManagerTest extends TestCase
@@ -95,6 +96,143 @@ public function getStrategyTests()
         ];
     }
 
+    public function testCacheableVoters()
+    {
+        $token = $this->createMock(TokenInterface::class);
+        $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass();
+        $voter
+            ->expects($this->once())
+            ->method('supportsAttribute')
+            ->with('foo')
+            ->willReturn(true);
+        $voter
+            ->expects($this->once())
+            ->method('supportsType')
+            ->with('string')
+            ->willReturn(true);
+        $voter
+            ->expects($this->once())
+            ->method('vote')
+            ->with($token, 'bar', ['foo'])
+            ->willReturn(VoterInterface::ACCESS_GRANTED);
+
+        $manager = new AccessDecisionManager([$voter]);
+        $this->assertTrue($manager->decide($token, ['foo'], 'bar'));
+    }
+
+    public function testCacheableVotersIgnoresNonStringAttributes()
+    {
+        $token = $this->createMock(TokenInterface::class);
+        $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass();
+        $voter
+            ->expects($this->never())
+            ->method('supportsAttribute');
+        $voter
+            ->expects($this->once())
+            ->method('supportsType')
+            ->with('string')
+            ->willReturn(true);
+        $voter
+            ->expects($this->once())
+            ->method('vote')
+            ->with($token, 'bar', [1337])
+            ->willReturn(VoterInterface::ACCESS_GRANTED);
+
+        $manager = new AccessDecisionManager([$voter]);
+        $this->assertTrue($manager->decide($token, [1337], 'bar'));
+    }
+
+    public function testCacheableVotersWithMultipleAttributes()
+    {
+        $token = $this->createMock(TokenInterface::class);
+        $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass();
+        $voter
+            ->expects($this->exactly(2))
+            ->method('supportsAttribute')
+            ->withConsecutive(['foo'], ['bar'])
+            ->willReturnOnConsecutiveCalls(false, true);
+        $voter
+            ->expects($this->once())
+            ->method('supportsType')
+            ->with('string')
+            ->willReturn(true);
+        $voter
+            ->expects($this->once())
+            ->method('vote')
+            ->with($token, 'bar', ['foo', 'bar'])
+            ->willReturn(VoterInterface::ACCESS_GRANTED);
+
+        $manager = new AccessDecisionManager([$voter]);
+        $this->assertTrue($manager->decide($token, ['foo', 'bar'], 'bar', true));
+    }
+
+    public function testCacheableVotersWithEmptyAttributes()
+    {
+        $token = $this->createMock(TokenInterface::class);
+        $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass();
+        $voter
+            ->expects($this->never())
+            ->method('supportsAttribute');
+        $voter
+            ->expects($this->once())
+            ->method('supportsType')
+            ->with('string')
+            ->willReturn(true);
+        $voter
+            ->expects($this->once())
+            ->method('vote')
+            ->with($token, 'bar', [])
+            ->willReturn(VoterInterface::ACCESS_GRANTED);
+
+        $manager = new AccessDecisionManager([$voter]);
+        $this->assertTrue($manager->decide($token, [], 'bar'));
+    }
+
+    public function testCacheableVotersSupportsMethodsCalledOnce()
+    {
+        $token = $this->createMock(TokenInterface::class);
+        $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass();
+        $voter
+            ->expects($this->once())
+            ->method('supportsAttribute')
+            ->with('foo')
+            ->willReturn(true);
+        $voter
+            ->expects($this->once())
+            ->method('supportsType')
+            ->with('string')
+            ->willReturn(true);
+        $voter
+            ->expects($this->exactly(2))
+            ->method('vote')
+            ->with($token, 'bar', ['foo'])
+            ->willReturn(VoterInterface::ACCESS_GRANTED);
+
+        $manager = new AccessDecisionManager([$voter]);
+        $this->assertTrue($manager->decide($token, ['foo'], 'bar'));
+        $this->assertTrue($manager->decide($token, ['foo'], 'bar'));
+    }
+
+    public function testCacheableVotersNotCalled()
+    {
+        $token = $this->createMock(TokenInterface::class);
+        $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass();
+        $voter
+            ->expects($this->once())
+            ->method('supportsAttribute')
+            ->with('foo')
+            ->willReturn(false);
+        $voter
+            ->expects($this->never())
+            ->method('supportsType');
+        $voter
+            ->expects($this->never())
+            ->method('vote');
+
+        $manager = new AccessDecisionManager([$voter]);
+        $this->assertFalse($manager->decide($token, ['foo'], 'bar'));
+    }
+
     protected function getVoters($grants, $denies, $abstains)
     {
         $voters = [];
diff --git a/Tests/Authorization/Voter/AuthenticatedVoterTest.php b/Tests/Authorization/Voter/AuthenticatedVoterTest.php
diff --git a/Tests/Authorization/Voter/RoleVoterTest.php b/Tests/Authorization/Voter/RoleVoterTest.php
diff --git a/Tests/Authorization/Voter/TraceableVoterTest.php b/Tests/Authorization/Voter/TraceableVoterTest.php

Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,7 @@`
`18`	`18`	`*`
`19`	`19`	`* @author Fabien Potencier <fabien@symfony.com>`
`20`	`20`	`*/`
`21`		`-class RoleVoter implements VoterInterface`
	`21`	`+class RoleVoter implements CacheableVoterInterface`
`22`	`22`	`{`
`23`	`23`	`private $prefix;`
`24`	`24`
`@@ -51,6 +51,16 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes):`
`51`	`51`	`return $result;`
`52`	`52`	`}`
`53`	`53`
	`54`	`+ public function supportsAttribute(string $attribute): bool`
	`55`	`+ {`
	`56`	`+ return str_starts_with($attribute, $this->prefix);`
	`57`	`+ }`
	`58`	`+`
	`59`	`+ public function supportsType(string $subjectType): bool`
	`60`	`+ {`
	`61`	`+ return true;`
	`62`	`+ }`
	`63`	`+`
`54`	`64`	`protected function extractRoles(TokenInterface $token)`
`55`	`65`	`{`
`56`	`66`	`return $token->getRoleNames();`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@`
`22`	`22`	`*`
`23`	`23`	`* @internal`
`24`	`24`	`*/`
`25`		`-class TraceableVoter implements VoterInterface`
	`25`	`+class TraceableVoter implements CacheableVoterInterface`
`26`	`26`	`{`
`27`	`27`	`private $voter;`
`28`	`28`	`private $eventDispatcher;`
`@@ -46,4 +46,14 @@ public function getDecoratedVoter(): VoterInterface`
`46`	`46`	`{`
`47`	`47`	`return $this->voter;`
`48`	`48`	`}`
	`49`	`+`
	`50`	`+ public function supportsAttribute(string $attribute): bool`
	`51`	`+ {`
	`52`	`+ return !$this->voter instanceof CacheableVoterInterface \|\| $this->voter->supportsAttribute($attribute);`
	`53`	`+ }`
	`54`	`+`
	`55`	`+ public function supportsType(string $subjectType): bool`
	`56`	`+ {`
	`57`	`+ return !$this->voter instanceof CacheableVoterInterface \|\| $this->voter->supportsType($subjectType);`
	`58`	`+ }`
`49`	`59`	`}`