Refactor access control system to use token-based requester.

Spomky · Spomky · commit 7212e071f200 · 2025-01-14T10:22:52.000+01:00
Replaced reliance on `TokenStorageInterface` with requester tokens directly passed via `AccessRequest` objects. Introduced `MetadataBag` for improved metadata handling and marked several classes as `final`. Updated tests and strategies accordingly to simplify the architecture and enhance maintainability.
diff --git a/src/Symfony/Component/AccessControl/AccessControlManager.php b/src/Symfony/Component/AccessControl/AccessControlManager.php
@@ -12,7 +12,7 @@
 /**
  * @experimental
  */
-class AccessControlManager implements AccessControlManagerInterface
+final class AccessControlManager implements AccessControlManagerInterface
 {
     private readonly string $defaultStrategy;
 
diff --git a/src/Symfony/Component/AccessControl/AccessRequest.php b/src/Symfony/Component/AccessControl/AccessRequest.php
@@ -4,19 +4,18 @@
 
 use Symfony\Component\ExpressionLanguage\Expression;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\User\UserInterface;
 
 /**
  * @experimental
  */
 readonly class AccessRequest
 {
-    /**
-     * @param array<array-key, mixed> $metadata
-     */
     public function __construct(
+        public null|TokenInterface $requester,
         public mixed $attribute,
         public mixed  $subject = null,
-        public array $metadata = [],
+        public MetadataBag $metadata = new MetadataBag(),
         public bool $allowIfAllAbstainOrTie = false,
     ) {
     }
diff --git a/src/Symfony/Component/AccessControl/Attribute/AccessPolicy.php b/src/Symfony/Component/AccessControl/Attribute/AccessPolicy.php
@@ -18,7 +18,7 @@
 readonly class AccessPolicy
 {
     /**
-     * @param array<array-key, mixed> $
+     * @param array<array-key, mixed> $metadata
      */
     public function __construct(
         public mixed $attribute,
diff --git a/src/Symfony/Component/AccessControl/Attribute/All.php b/src/Symfony/Component/AccessControl/Attribute/All.php
@@ -15,7 +15,7 @@
  * @experimental
  */
 #[\Attribute(\Attribute::IS_REPEATABLE | \Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD | \Attribute::TARGET_FUNCTION)]
-readonly class All
+final readonly class All
 {
     /**
      * @param list<AccessPolicy> $accessPolicies
diff --git a/src/Symfony/Component/AccessControl/Attribute/AtLeastOneOf.php b/src/Symfony/Component/AccessControl/Attribute/AtLeastOneOf.php
@@ -15,7 +15,7 @@
  * @experimental
  */
 #[\Attribute(\Attribute::IS_REPEATABLE | \Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD | \Attribute::TARGET_FUNCTION)]
-readonly class AtLeastOneOf
+final readonly class AtLeastOneOf
 {
     /**
      * @param list<AccessPolicy> $accessPolicies
diff --git a/src/Symfony/Component/AccessControl/Exception/InvalidStrategyException.php b/src/Symfony/Component/AccessControl/Exception/InvalidStrategyException.php
@@ -5,7 +5,7 @@
 /**
  * @experimental
  */
-class InvalidStrategyException extends \RuntimeException
+final class InvalidStrategyException extends \RuntimeException
 {
 
 }
diff --git a/src/Symfony/Component/AccessControl/Listener/AccessPolicyListener.php b/src/Symfony/Component/AccessControl/Listener/AccessPolicyListener.php
@@ -6,12 +6,14 @@
 use Symfony\Component\AccessControl\AccessControlManager;
 use Symfony\Component\AccessControl\Attribute\AccessPolicy;
 use Symfony\Component\AccessControl\DecisionVote;
+use Symfony\Component\AccessControl\MetadataBag;
 use Symfony\Component\Console\ConsoleEvents;
 use Symfony\Component\Console\Event\ConsoleCommandEvent;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent;
 use Symfony\Component\HttpKernel\Exception\HttpException;
 use Symfony\Component\HttpKernel\KernelEvents;
+use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
 use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 
 /**
@@ -20,6 +22,7 @@
 final readonly class AccessPolicyListener implements EventSubscriberInterface
 {
     public function __construct(
+        private TokenStorageInterface $tokenStorage,
         private AccessControlManager $accessControlManager,
     ) {
     }
@@ -73,12 +76,13 @@ public function onKernelControllerArguments(ControllerArgumentsEvent $event): vo
     private function processAttribute(AccessPolicy $attribute, array $metadata): void
     {
         $accessRequest = new AccessRequest(
+            $this->tokenStorage->getToken(),
             $attribute->attribute,
             $attribute->subject,
-            [
+            new MetadataBag([
                 ...$attribute->metadata,
                 ...$metadata,
-            ],
+            ]),
             $attribute->allowIfAllAbstain
         );
         $accessDecision = $this->accessControlManager->decide($accessRequest, $attribute->strategy);
diff --git a/src/Symfony/Component/AccessControl/Listener/AllListener.php b/src/Symfony/Component/AccessControl/Listener/AllListener.php
@@ -6,12 +6,14 @@
 use Symfony\Component\AccessControl\AccessControlManager;
 use Symfony\Component\AccessControl\Attribute\All;
 use Symfony\Component\AccessControl\DecisionVote;
+use Symfony\Component\AccessControl\MetadataBag;
 use Symfony\Component\Console\ConsoleEvents;
 use Symfony\Component\Console\Event\ConsoleCommandEvent;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent;
 use Symfony\Component\HttpKernel\Exception\HttpException;
 use Symfony\Component\HttpKernel\KernelEvents;
+use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
 use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 
 /**
@@ -20,6 +22,7 @@
 final readonly class AllListener implements EventSubscriberInterface
 {
     public function __construct(
+        private TokenStorageInterface $tokenStorage,
         private AccessControlManager $accessControlManager,
     ) {
     }
@@ -74,12 +77,13 @@ private function processAttribute(All $attribute, array $metadata): void
     {
         foreach ($attribute->accessPolicies as $accessPolicy) {
             $accessRequest = new AccessRequest(
+                $this->tokenStorage->getToken(),
                 $accessPolicy->attribute,
                 $accessPolicy->subject,
-                [
+                new MetadataBag([
                     ...$accessPolicy->metadata,
                     ...$metadata,
-                ],
+                ]),
                 $accessPolicy->allowIfAllAbstain
             );
             $accessDecision = $this->accessControlManager->decide($accessRequest, $accessPolicy->strategy);
diff --git a/src/Symfony/Component/AccessControl/Listener/AtLeastOneOfListener.php b/src/Symfony/Component/AccessControl/Listener/AtLeastOneOfListener.php
@@ -6,12 +6,14 @@
 use Symfony\Component\AccessControl\AccessControlManager;
 use Symfony\Component\AccessControl\Attribute\All;
 use Symfony\Component\AccessControl\DecisionVote;
+use Symfony\Component\AccessControl\MetadataBag;
 use Symfony\Component\Console\ConsoleEvents;
 use Symfony\Component\Console\Event\ConsoleCommandEvent;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent;
 use Symfony\Component\HttpKernel\Exception\HttpException;
 use Symfony\Component\HttpKernel\KernelEvents;
+use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
 use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 
 /**
@@ -20,6 +22,7 @@
 final readonly class AtLeastOneOfListener implements EventSubscriberInterface
 {
     public function __construct(
+        private TokenStorageInterface $tokenStorage,
         private AccessControlManager $accessControlManager,
     ) {
     }
@@ -74,12 +77,13 @@ private function processAttribute(All $attribute, array $metadata): void
     {
         foreach ($attribute->accessPolicies as $accessPolicy) {
             $accessRequest = new AccessRequest(
+                $this->tokenStorage->getToken(),
                 $accessPolicy->attribute,
                 $accessPolicy->subject,
-                [
+                new MetadataBag([
                     ...$accessPolicy->metadata,
                     ...$metadata,
-                ],
+                ]),
                 $accessPolicy->allowIfAllAbstain
             );
             $accessDecision = $this->accessControlManager->decide($accessRequest, $accessPolicy->strategy);
diff --git a/src/Symfony/Component/AccessControl/MetadataBag.php b/src/Symfony/Component/AccessControl/MetadataBag.php
@@ -0,0 +1,56 @@
+<?php
+
+namespace Symfony\Component\AccessControl;
+
+/**
+ * @experimental
+ */
+final readonly class MetadataBag implements \IteratorAggregate, \Countable
+{
+    /**
+     * @param array<string, mixed> $parameters
+     */
+    public function __construct(
+        private array $parameters = [],
+    ) {
+    }
+
+    /**
+     * Returns the parameter keys.
+     */
+    public function keys(): array
+    {
+        return array_keys($this->parameters);
+    }
+
+    public function get(string $key, mixed $default = null): mixed
+    {
+        return \array_key_exists($key, $this->parameters) ? $this->parameters[$key] : $default;
+    }
+
+    /**
+     * Returns true if the parameter is defined.
+     */
+    public function has(string $key): bool
+    {
+        return \array_key_exists($key, $this->parameters);
+    }
+
+    /**
+     * Returns an iterator for parameters.
+     *
+     * @return \ArrayIterator<string, mixed>
+     */
+    public function getIterator(): \ArrayIterator
+    {
+        return new \ArrayIterator($this->parameters);
+    }
+
+    /**
+     * Returns the number of parameters.
+     */
+    public function count(): int
+    {
+        return \count($this->parameters);
+    }
+}
diff --git a/src/Symfony/Component/AccessControl/Tests/AffirmativeStrategyTest.php b/src/Symfony/Component/AccessControl/Tests/AffirmativeStrategyTest.php
@@ -13,10 +13,9 @@ final class AffirmativeStrategyTest extends StrategyTestCase
     /**
      * @dataProvider provideScenarios
      */
-    public function testDecide(?TokenInterface $token, AccessRequest $accessRequest, DecisionVote $expectedDecision, ?string $reason): void
+    public function testDecide(AccessRequest $accessRequest, DecisionVote $expectedDecision, ?string $reason): void
     {
         // Arrange
-        $this->getTokenStorage()->setToken($token);
         $accessControlManger = $this->getAccessControlManager();
 
         // Act
@@ -33,57 +32,50 @@ public function testDecide(?TokenInterface $token, AccessRequest $accessRequest,
     public function provideScenarios(): iterable
     {
         yield 'affirmative strategy and deny on abstain' => [
-            new NullToken(),
-            new AccessRequest('read', 'article'),
+            new AccessRequest(new NullToken(),'read', 'article'),
             DecisionVote::ACCESS_DENIED,
             'All voters abstained from voting.',
         ];
 
         yield 'affirmative strategy and grant on abstain' => [
-            new NullToken(),
-            new AccessRequest('read', 'article', allowIfAllAbstainOrTie: true),
+            new AccessRequest(new NullToken(),'read', 'article', allowIfAllAbstainOrTie: true),
             DecisionVote::ACCESS_GRANTED,
             'All voters abstained from voting.',
         ];
         yield 'affirmative strategy and deny on unauthenticated user' => [
-            new NullToken(),
-            new AccessRequest('ROLE_USER'),
+            new AccessRequest(new NullToken(),'ROLE_USER'),
             DecisionVote::ACCESS_DENIED,
             'At least one voter denied access.',
         ];
 
         $userToken = $this->createMock(TokenInterface::class);
         $userToken->method('getUser')->willReturn(new FakeUser);
         yield 'affirmative strategy and grant on authenticated user (classic interface)' => [
-            $userToken,
-            new AccessRequest('ROLE_USER'),
+            new AccessRequest($userToken,'ROLE_USER'),
             DecisionVote::ACCESS_GRANTED,
             'The user has the required role.',
         ];
 
         $userToken = $this->createMock(TokenInterface::class);
         $userToken->method('getUser')->willReturn(new FakeUserWithRole);
         yield 'affirmative strategy and grant on authenticated user (new interface)' => [
-            $userToken,
-            new AccessRequest('ROLE_USER'),
+            new AccessRequest($userToken,'ROLE_USER'),
             DecisionVote::ACCESS_GRANTED,
             'The user has the required role.',
         ];
 
         $userToken = $this->createMock(TokenInterface::class);
         $userToken->method('getUser')->willReturn(new FakeUserWithRole(roles: ['ROLE_SUPER_ADMIN']));
         yield 'affirmative strategy and grant on authenticated user (inherited role)' => [
-            $userToken,
-            new AccessRequest('ROLE_ALLOWED_TO_SWITCH'),
+            new AccessRequest($userToken,'ROLE_ALLOWED_TO_SWITCH'),
             DecisionVote::ACCESS_GRANTED,
             'The user has the required role.',
         ];
 
         $userToken = $this->createMock(TokenInterface::class);
         $userToken->method('getUser')->willReturn(new FakeUserWithRole(roles: ['ROLE_ADMIN']));
         yield 'affirmative strategy and deny on authenticated user (inherited role)' => [
-            $userToken,
-            new AccessRequest('ROLE_ALLOWED_TO_SWITCH'),
+            new AccessRequest($userToken,'ROLE_ALLOWED_TO_SWITCH'),
             DecisionVote::ACCESS_DENIED,
             'At least one voter denied access.',
         ];
@@ -92,8 +84,7 @@ public function provideScenarios(): iterable
         $userToken->method('getUser')->willReturn(new FakeUserWithRole(roles: ['ROLE_ADMIN']));
         $expression = new Expression('"ROLE_ADMIN" in role_names and is_authenticated()');
         yield 'affirmative strategy and grant on expression' => [
-            $userToken,
-            new AccessRequest($expression),
+            new AccessRequest($userToken,$expression),
             DecisionVote::ACCESS_GRANTED,
             'Access granted by expression',
         ];
@@ -102,8 +93,7 @@ public function provideScenarios(): iterable
         $userToken->method('getUser')->willReturn(new FakeUserWithRole(roles: ['ROLE_ADMIN']));
         $expression = new Expression('"ROLE_SUPER_ADMIN" in role_names and is_fully_authenticated()');
         yield 'affirmative strategy and denied on expression' => [
-            $userToken,
-            new AccessRequest($expression),
+            new AccessRequest($userToken,$expression),
             DecisionVote::ACCESS_DENIED,
             'At least one voter denied access.',
         ];
diff --git a/src/Symfony/Component/AccessControl/Tests/ConsensusStrategyTest.php b/src/Symfony/Component/AccessControl/Tests/ConsensusStrategyTest.php
@@ -11,10 +11,9 @@ final class ConsensusStrategyTest extends StrategyTestCase
     /**
      * @dataProvider provideScenarios
      */
-    public function testDecide(?TokenInterface $token, AccessRequest $accessRequest, DecisionVote $expectedDecision, ?string $reason): void
+    public function testDecide(AccessRequest $accessRequest, DecisionVote $expectedDecision, ?string $reason): void
     {
         // Arrange
-        $this->getTokenStorage()->setToken($token);
         $accessControlManger = $this->getAccessControlManager();
 
         // Act
@@ -33,17 +32,15 @@ public function provideScenarios(): iterable
         $userToken = $this->createMock(TokenInterface::class);
         $userToken->method('getUser')->willReturn(new FakeUserWithRole(roles: ['ROLE_SUPER_ADMIN']));
         yield 'consensus strategy and deny on authenticated user' => [
-            $userToken,
-            new AccessRequest('ROLE_ALLOWED_TO_SWITCH'),
+            new AccessRequest($userToken,'ROLE_ALLOWED_TO_SWITCH'),
             DecisionVote::ACCESS_DENIED,
             'There is a tie.',
         ];
 
         $userToken = $this->createMock(TokenInterface::class);
         $userToken->method('getUser')->willReturn(new FakeUserWithRole(roles: ['ROLE_SUPER_ADMIN']));
         yield 'consensus strategy and grant on authenticated user' => [
-            $userToken,
-            new AccessRequest('ROLE_ALLOWED_TO_SWITCH', allowIfAllAbstainOrTie: true),
+            new AccessRequest($userToken,'ROLE_ALLOWED_TO_SWITCH', allowIfAllAbstainOrTie: true),
             DecisionVote::ACCESS_GRANTED,
             'There is a tie.',
         ];
diff --git a/src/Symfony/Component/AccessControl/Tests/EventsTest.php b/src/Symfony/Component/AccessControl/Tests/EventsTest.php
@@ -12,8 +12,7 @@ final class EventsTest extends StrategyTestCase
     public function testDecide(): void
     {
         // Arrange
-        $this->getTokenStorage()->setToken(new NullToken());
-        $accessRequest = new AccessRequest('PUBLIC_ACCESS');
+        $accessRequest = new AccessRequest(new NullToken(), 'PUBLIC_ACCESS');
 
         // Act
         $this->getAccessControlManager()->decide($accessRequest);
diff --git a/src/Symfony/Component/AccessControl/Tests/StrategyTestCase.php b/src/Symfony/Component/AccessControl/Tests/StrategyTestCase.php
@@ -45,11 +45,10 @@ protected function getAccessControlManager(): AccessControlManager
                     new ExpressionVoter(
                         new ExpressionLanguage(),
                         new AuthenticationTrustResolver(),
-                        $this->getTokenStorage(),
                         $this->getRoleHierarchy(),
                     ),
-                    new RoleVoter($this->getTokenStorage()),
-                    new RoleHierarchyVoter($this->getRoleHierarchy(), $this->getTokenStorage()),
+                    new RoleVoter(),
+                    new RoleHierarchyVoter($this->getRoleHierarchy()),
                     new AuthenticatedVoter(
                         new AuthenticationTrustResolver(),
                         $this->getTokenStorage()
diff --git a/src/Symfony/Component/AccessControl/Tests/UnanimousStrategyTest.php b/src/Symfony/Component/AccessControl/Tests/UnanimousStrategyTest.php
diff --git a/src/Symfony/Component/AccessControl/Voter/ABAC/AuthenticatedVoter.php b/src/Symfony/Component/AccessControl/Voter/ABAC/AuthenticatedVoter.php
diff --git a/src/Symfony/Component/AccessControl/Voter/Expression/ExpressionVoter.php b/src/Symfony/Component/AccessControl/Voter/Expression/ExpressionVoter.php
diff --git a/src/Symfony/Component/AccessControl/Voter/RBAC/RoleHierarchyVoter.php b/src/Symfony/Component/AccessControl/Voter/RBAC/RoleHierarchyVoter.php
diff --git a/src/Symfony/Component/AccessControl/Voter/RBAC/RoleVoter.php b/src/Symfony/Component/AccessControl/Voter/RBAC/RoleVoter.php

Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@`
`12`	`12`	`/**`
`13`	`13`	`* @experimental`
`14`	`14`	`*/`
`15`		`-class AccessControlManager implements AccessControlManagerInterface`
	`15`	`+final class AccessControlManager implements AccessControlManagerInterface`
`16`	`16`	`{`
`17`	`17`	`private readonly string $defaultStrategy;`
`18`	`18`
Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,7 @@`
`18`	`18`	`readonly class AccessPolicy`
`19`	`19`	`{`
`20`	`20`	`/**`
`21`		`- * @param array<array-key, mixed> $`
	`21`	`+ * @param array<array-key, mixed> $metadata`
`22`	`22`	`*/`
`23`	`23`	`public function __construct(`
`24`	`24`	`public mixed $attribute,`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@`
`15`	`15`	`* @experimental`
`16`	`16`	`*/`
`17`	`17`	`#[\Attribute(\Attribute::IS_REPEATABLE \| \Attribute::TARGET_CLASS \| \Attribute::TARGET_METHOD \| \Attribute::TARGET_FUNCTION)]`
`18`		`-readonly class All`
	`18`	`+final readonly class All`
`19`	`19`	`{`
`20`	`20`	`/**`
`21`	`21`	`* @param list<AccessPolicy> $accessPolicies`
Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,7 @@`
`5`	`5`	`/**`
`6`	`6`	`* @experimental`
`7`	`7`	`*/`
`8`		`-class InvalidStrategyException extends \RuntimeException`
	`8`	`+final class InvalidStrategyException extends \RuntimeException`
`9`	`9`	`{`
`10`	`10`
`11`	`11`	`}`