From 78478b0c563999eb67fdcfc7ea0462c8c452dbde Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 23 May 2023 17:24:39 +0200 Subject: [PATCH 01/52] [7.0] Bump to PHP 8.2 minimum --- composer.json | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/composer.json b/composer.json index ba1c62fa..0bc2c140 100644 --- a/composer.json +++ b/composer.json @@ -16,30 +16,30 @@ } ], "require": { - "php": ">=8.1", + "php": ">=8.2", "symfony/event-dispatcher-contracts": "^2.5|^3", "symfony/service-contracts": "^2.5|^3", - "symfony/password-hasher": "^5.4|^6.0|^7.0" + "symfony/password-hasher": "^6.4|^7.0" }, "require-dev": { "psr/container": "^1.1|^2.0", "psr/cache": "^1.0|^2.0|^3.0", - "symfony/cache": "^5.4|^6.0|^7.0", - "symfony/event-dispatcher": "^5.4|^6.0|^7.0", - "symfony/expression-language": "^5.4|^6.0|^7.0", - "symfony/http-foundation": "^5.4|^6.0|^7.0", - "symfony/ldap": "^5.4|^6.0|^7.0", - "symfony/string": "^5.4|^6.0|^7.0", - "symfony/translation": "^5.4|^6.0|^7.0", - "symfony/validator": "^5.4|^6.0|^7.0", + "symfony/cache": "^6.4|^7.0", + "symfony/event-dispatcher": "^6.4|^7.0", + "symfony/expression-language": "^6.4|^7.0", + "symfony/http-foundation": "^6.4|^7.0", + "symfony/ldap": "^6.4|^7.0", + "symfony/string": "^6.4|^7.0", + "symfony/translation": "^6.4|^7.0", + "symfony/validator": "^6.4|^7.0", "psr/log": "^1|^2|^3" }, "conflict": { - "symfony/event-dispatcher": "<5.4", - "symfony/http-foundation": "<5.4", - "symfony/security-guard": "<5.4", - "symfony/ldap": "<5.4", - "symfony/validator": "<5.4" + "symfony/event-dispatcher": "<6.4", + "symfony/http-foundation": "<6.4", + "symfony/security-guard": "<6.4", + "symfony/ldap": "<6.4", + "symfony/validator": "<6.4" }, "autoload": { "psr-4": { "Symfony\\Component\\Security\\Core\\": "" }, From bf58846b673cbfeb597360c8c8fb6096afd0f214 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 30 Jun 2023 10:51:15 +0200 Subject: [PATCH 02/52] [7.0] Fix various `@psalm-return` annotations --- Authorization/Voter/VoterInterface.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Authorization/Voter/VoterInterface.php b/Authorization/Voter/VoterInterface.php index 8eea57e7..5255c88e 100644 --- a/Authorization/Voter/VoterInterface.php +++ b/Authorization/Voter/VoterInterface.php @@ -33,9 +33,7 @@ interface VoterInterface * @param mixed $subject The subject to secure * @param array $attributes An array of attributes associated with the method being invoked * - * @return int either ACCESS_GRANTED, ACCESS_ABSTAIN, or ACCESS_DENIED - * - * @psalm-return self::ACCESS_* must be transformed into @return on Symfony 7 + * @return self::ACCESS_* */ - public function vote(TokenInterface $token, mixed $subject, array $attributes); + public function vote(TokenInterface $token, mixed $subject, array $attributes): int; } From 3c7de5601a92acae8f4279d8bc9fdbd1f8fd3091 Mon Sep 17 00:00:00 2001 From: Frank Fiebig Date: Tue, 4 Jul 2023 11:21:50 +0200 Subject: [PATCH 03/52] [Lock] 7.0 remove deprecations in Lock Component --- Authentication/Token/Storage/TokenStorage.php | 6 +- Authorization/AuthorizationChecker.php | 15 +-- Security.php | 11 --- .../Token/Storage/TokenStorageTest.php | 19 ---- Tests/SecurityTest.php | 98 ------------------- User/ChainUserProvider.php | 8 -- User/InMemoryUserProvider.php | 10 +- 7 files changed, 8 insertions(+), 159 deletions(-) delete mode 100644 Tests/SecurityTest.php diff --git a/Authentication/Token/Storage/TokenStorage.php b/Authentication/Token/Storage/TokenStorage.php index 0ec6b1cf..8acc31bc 100644 --- a/Authentication/Token/Storage/TokenStorage.php +++ b/Authentication/Token/Storage/TokenStorage.php @@ -40,12 +40,8 @@ public function getToken(): ?TokenInterface /** * @return void */ - public function setToken(TokenInterface $token = null) + public function setToken(?TokenInterface $token) { - if (1 > \func_num_args()) { - trigger_deprecation('symfony/security-core', '6.2', 'Calling "%s()" without any arguments is deprecated, pass null explicitly instead.', __METHOD__); - } - if ($token) { // ensure any initializer is called $this->getToken(); diff --git a/Authorization/AuthorizationChecker.php b/Authorization/AuthorizationChecker.php index 3827f8b9..c748697c 100644 --- a/Authorization/AuthorizationChecker.php +++ b/Authorization/AuthorizationChecker.php @@ -24,17 +24,10 @@ */ class AuthorizationChecker implements AuthorizationCheckerInterface { - private TokenStorageInterface $tokenStorage; - private AccessDecisionManagerInterface $accessDecisionManager; - - public function __construct(TokenStorageInterface $tokenStorage, AccessDecisionManagerInterface $accessDecisionManager, bool $exceptionOnNoToken = false) - { - if ($exceptionOnNoToken) { - throw new \LogicException(sprintf('Argument $exceptionOnNoToken of "%s()" must be set to "false".', __METHOD__)); - } - - $this->tokenStorage = $tokenStorage; - $this->accessDecisionManager = $accessDecisionManager; + public function __construct( + private TokenStorageInterface $tokenStorage, + private AccessDecisionManagerInterface $accessDecisionManager, + ) { } final public function isGranted(mixed $attribute, mixed $subject = null): bool diff --git a/Security.php b/Security.php index 97f1c8ce..bb2576a7 100644 --- a/Security.php +++ b/Security.php @@ -24,19 +24,8 @@ */ class Security implements AuthorizationCheckerInterface { - /** - * @deprecated since Symfony 6.2, use \Symfony\Bundle\SecurityBundle\Security::ACCESS_DENIED_ERROR instead - */ public const ACCESS_DENIED_ERROR = '_security.403_error'; - - /** - * @deprecated since Symfony 6.2, use \Symfony\Bundle\SecurityBundle\Security::AUTHENTICATION_ERROR instead - */ public const AUTHENTICATION_ERROR = '_security.last_error'; - - /** - * @deprecated since Symfony 6.2, use \Symfony\Bundle\SecurityBundle\Security::LAST_USERNAME instead - */ public const LAST_USERNAME = '_security.last_username'; /** diff --git a/Tests/Authentication/Token/Storage/TokenStorageTest.php b/Tests/Authentication/Token/Storage/TokenStorageTest.php index 5b260b50..26d20ae4 100644 --- a/Tests/Authentication/Token/Storage/TokenStorageTest.php +++ b/Tests/Authentication/Token/Storage/TokenStorageTest.php @@ -12,31 +12,12 @@ namespace Symfony\Component\Security\Core\Tests\Authentication\Token\Storage; use PHPUnit\Framework\TestCase; -use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\User\InMemoryUser; class TokenStorageTest extends TestCase { - use ExpectDeprecationTrait; - - /** - * @group legacy - */ - public function testGetSetTokenLegacy() - { - $tokenStorage = new TokenStorage(); - $token = new UsernamePasswordToken(new InMemoryUser('username', 'password'), 'provider'); - $tokenStorage->setToken($token); - $this->assertSame($token, $tokenStorage->getToken()); - - $this->expectDeprecation('Since symfony/security-core 6.2: Calling "Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage::setToken()" without any arguments is deprecated, pass null explicitly instead.'); - - $tokenStorage->setToken(); - $this->assertNull($tokenStorage->getToken()); - } - public function testGetSetToken() { $tokenStorage = new TokenStorage(); diff --git a/Tests/SecurityTest.php b/Tests/SecurityTest.php deleted file mode 100644 index 00436895..00000000 --- a/Tests/SecurityTest.php +++ /dev/null @@ -1,98 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Security\Core\Tests; - -use PHPUnit\Framework\TestCase; -use Psr\Container\ContainerInterface; -use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; -use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; -use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; -use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; -use Symfony\Component\Security\Core\Security; -use Symfony\Component\Security\Core\User\InMemoryUser; - -/** - * @group legacy - */ -class SecurityTest extends TestCase -{ - public function testGetToken() - { - $token = new UsernamePasswordToken(new InMemoryUser('foo', 'bar'), 'provider'); - $tokenStorage = $this->createMock(TokenStorageInterface::class); - - $tokenStorage->expects($this->once()) - ->method('getToken') - ->willReturn($token); - - $container = $this->createContainer('security.token_storage', $tokenStorage); - - $security = new Security($container); - $this->assertSame($token, $security->getToken()); - } - - /** - * @dataProvider getUserTests - */ - public function testGetUser($userInToken, $expectedUser) - { - $token = $this->createMock(TokenInterface::class); - $token->expects($this->any()) - ->method('getUser') - ->willReturn($userInToken); - $tokenStorage = $this->createMock(TokenStorageInterface::class); - - $tokenStorage->expects($this->once()) - ->method('getToken') - ->willReturn($token); - - $container = $this->createContainer('security.token_storage', $tokenStorage); - - $security = new Security($container); - $this->assertSame($expectedUser, $security->getUser()); - } - - public static function getUserTests() - { - yield [null, null]; - - $user = new InMemoryUser('nice_user', 'foo'); - yield [$user, $user]; - } - - public function testIsGranted() - { - $authorizationChecker = $this->createMock(AuthorizationCheckerInterface::class); - - $authorizationChecker->expects($this->once()) - ->method('isGranted') - ->with('SOME_ATTRIBUTE', 'SOME_SUBJECT') - ->willReturn(true); - - $container = $this->createContainer('security.authorization_checker', $authorizationChecker); - - $security = new Security($container); - $this->assertTrue($security->isGranted('SOME_ATTRIBUTE', 'SOME_SUBJECT')); - } - - private function createContainer($serviceId, $serviceObject) - { - $container = $this->createMock(ContainerInterface::class); - - $container->expects($this->atLeastOnce()) - ->method('get') - ->with($serviceId) - ->willReturn($serviceObject); - - return $container; - } -} diff --git a/User/ChainUserProvider.php b/User/ChainUserProvider.php index 47ebbc1a..045697fb 100644 --- a/User/ChainUserProvider.php +++ b/User/ChainUserProvider.php @@ -46,14 +46,6 @@ public function getProviders(): array return $this->providers; } - /** - * @internal for compatibility with Symfony 5.4 - */ - public function loadUserByUsername(string $username): UserInterface - { - return $this->loadUserByIdentifier($username); - } - public function loadUserByIdentifier(string $identifier): UserInterface { foreach ($this->providers as $provider) { diff --git a/User/InMemoryUserProvider.php b/User/InMemoryUserProvider.php index e0aef90a..13441bc7 100644 --- a/User/InMemoryUserProvider.php +++ b/User/InMemoryUserProvider.php @@ -51,13 +51,11 @@ public function __construct(array $users = []) * Adds a new User to the provider. * * @return void - * - * @throws \LogicException */ public function createUser(UserInterface $user) { if (!$user instanceof InMemoryUser) { - trigger_deprecation('symfony/security-core', '6.3', 'Passing users that are not instance of "%s" to "%s" is deprecated, "%s" given.', InMemoryUser::class, __METHOD__, get_debug_type($user)); + throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_debug_type($user))); } $userIdentifier = strtolower($user->getUserIdentifier()); @@ -93,13 +91,11 @@ public function supportsClass(string $class): bool } /** - * Returns the user by given username. - * - * @return InMemoryUser change return type on 7.0 + * Returns the user by given user. * * @throws UserNotFoundException if user whose given username does not exist */ - private function getUser(string $username): UserInterface + private function getUser(string $username): InMemoryUser { if (!isset($this->users[strtolower($username)])) { $ex = new UserNotFoundException(sprintf('Username "%s" does not exist.', $username)); From d888d4a7f9ffff7cb0b969ea4ee7abd00e4b8e4b Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 4 Jul 2023 14:50:59 +0200 Subject: [PATCH 04/52] [7.0] Remove remaining deprecated code paths --- CHANGELOG.md | 6 +++++ Security.php | 69 ---------------------------------------------------- 2 files changed, 6 insertions(+), 69 deletions(-) delete mode 100644 Security.php diff --git a/CHANGELOG.md b/CHANGELOG.md index b489556c..663b5999 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ CHANGELOG ========= +7.0 +--- + + * Remove the `Security` class, use `Symfony\Bundle\SecurityBundle\Security` instead + * Require explicit argument when calling `TokenStorage::setToken()` + 6.3 --- diff --git a/Security.php b/Security.php deleted file mode 100644 index bb2576a7..00000000 --- a/Security.php +++ /dev/null @@ -1,69 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Security\Core; - -use Psr\Container\ContainerInterface; -use Symfony\Bundle\SecurityBundle\Security as NewSecurityHelper; -use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; -use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; -use Symfony\Component\Security\Core\User\UserInterface; - -/** - * Helper class for commonly-needed security tasks. - * - * @deprecated since Symfony 6.2, use \Symfony\Bundle\SecurityBundle\Security instead - */ -class Security implements AuthorizationCheckerInterface -{ - public const ACCESS_DENIED_ERROR = '_security.403_error'; - public const AUTHENTICATION_ERROR = '_security.last_error'; - public const LAST_USERNAME = '_security.last_username'; - - /** - * @deprecated since Symfony 6.2, use \Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge::MAX_USERNAME_LENGTH instead - */ - public const MAX_USERNAME_LENGTH = 4096; - - private ContainerInterface $container; - - public function __construct(ContainerInterface $container, bool $triggerDeprecation = true) - { - $this->container = $container; - - if ($triggerDeprecation) { - trigger_deprecation('symfony/security-core', '6.2', 'The "%s" class is deprecated, use "%s" instead.', __CLASS__, NewSecurityHelper::class); - } - } - - public function getUser(): ?UserInterface - { - if (!$token = $this->getToken()) { - return null; - } - - return $token->getUser(); - } - - /** - * Checks if the attributes are granted against the current authentication token and optionally supplied subject. - */ - public function isGranted(mixed $attributes, mixed $subject = null): bool - { - return $this->container->get('security.authorization_checker') - ->isGranted($attributes, $subject); - } - - public function getToken(): ?TokenInterface - { - return $this->container->get('security.token_storage')->getToken(); - } -} From e267f60fb5549032781a48ec6c4fb6ce6f73ebdb Mon Sep 17 00:00:00 2001 From: Wouter de Jong Date: Sun, 2 Jul 2023 23:52:21 +0200 Subject: [PATCH 05/52] [Components] Convert to native return types --- .../RememberMe/InMemoryTokenProvider.php | 15 +++----------- .../RememberMe/TokenProviderInterface.php | 16 ++++----------- Authentication/Token/AbstractToken.php | 20 ++++--------------- Authentication/Token/NullToken.php | 20 ++++--------------- Authentication/Token/Storage/TokenStorage.php | 10 ++-------- .../Token/Storage/TokenStorageInterface.php | 4 +--- Authentication/Token/TokenInterface.php | 17 ++++------------ Authorization/Voter/RoleHierarchyVoter.php | 5 +---- Authorization/Voter/RoleVoter.php | 5 +---- Event/AuthenticationEvent.php | 5 +---- Exception/AccessDeniedException.php | 10 ++-------- Exception/AccountStatusException.php | 5 +---- Exception/AuthenticationException.php | 9 ++------- ...ustomUserMessageAccountStatusException.php | 4 +--- ...stomUserMessageAuthenticationException.php | 4 +--- Role/RoleHierarchy.php | 5 +---- User/InMemoryUserChecker.php | 10 ++-------- User/InMemoryUserProvider.php | 4 +--- User/UserCheckerInterface.php | 8 ++------ User/UserInterface.php | 4 +--- User/UserProviderInterface.php | 8 ++------ .../Constraints/UserPasswordValidator.php | 5 +---- 22 files changed, 42 insertions(+), 151 deletions(-) diff --git a/Authentication/RememberMe/InMemoryTokenProvider.php b/Authentication/RememberMe/InMemoryTokenProvider.php index 341883f3..1cc6ded5 100644 --- a/Authentication/RememberMe/InMemoryTokenProvider.php +++ b/Authentication/RememberMe/InMemoryTokenProvider.php @@ -31,10 +31,7 @@ public function loadTokenBySeries(string $series): PersistentTokenInterface return $this->tokens[$series]; } - /** - * @return void - */ - public function updateToken(string $series, #[\SensitiveParameter] string $tokenValue, \DateTime $lastUsed) + public function updateToken(string $series, #[\SensitiveParameter] string $tokenValue, \DateTime $lastUsed): void { if (!isset($this->tokens[$series])) { throw new TokenNotFoundException('No token found.'); @@ -50,18 +47,12 @@ public function updateToken(string $series, #[\SensitiveParameter] string $token $this->tokens[$series] = $token; } - /** - * @return void - */ - public function deleteTokenBySeries(string $series) + public function deleteTokenBySeries(string $series): void { unset($this->tokens[$series]); } - /** - * @return void - */ - public function createNewToken(PersistentTokenInterface $token) + public function createNewToken(PersistentTokenInterface $token): void { $this->tokens[$token->getSeries()] = $token; } diff --git a/Authentication/RememberMe/TokenProviderInterface.php b/Authentication/RememberMe/TokenProviderInterface.php index cf41e0c5..8be82c26 100644 --- a/Authentication/RememberMe/TokenProviderInterface.php +++ b/Authentication/RememberMe/TokenProviderInterface.php @@ -23,32 +23,24 @@ interface TokenProviderInterface /** * Loads the active token for the given series. * - * @return PersistentTokenInterface - * * @throws TokenNotFoundException if the token is not found */ - public function loadTokenBySeries(string $series); + public function loadTokenBySeries(string $series): PersistentTokenInterface; /** * Deletes all tokens belonging to series. - * - * @return void */ - public function deleteTokenBySeries(string $series); + public function deleteTokenBySeries(string $series): void; /** * Updates the token according to this data. * - * @return void - * * @throws TokenNotFoundException if the token is not found */ - public function updateToken(string $series, #[\SensitiveParameter] string $tokenValue, \DateTime $lastUsed); + public function updateToken(string $series, #[\SensitiveParameter] string $tokenValue, \DateTime $lastUsed): void; /** * Creates a new token. - * - * @return void */ - public function createNewToken(PersistentTokenInterface $token); + public function createNewToken(PersistentTokenInterface $token): void; } diff --git a/Authentication/Token/AbstractToken.php b/Authentication/Token/AbstractToken.php index 10cb8f77..36d64766 100644 --- a/Authentication/Token/AbstractToken.php +++ b/Authentication/Token/AbstractToken.php @@ -53,18 +53,12 @@ public function getUser(): ?UserInterface return $this->user; } - /** - * @return void - */ - public function setUser(UserInterface $user) + public function setUser(UserInterface $user): void { $this->user = $user; } - /** - * @return void - */ - public function eraseCredentials() + public function eraseCredentials(): void { if ($this->getUser() instanceof UserInterface) { $this->getUser()->eraseCredentials(); @@ -118,10 +112,7 @@ public function getAttributes(): array return $this->attributes; } - /** - * @return void - */ - public function setAttributes(array $attributes) + public function setAttributes(array $attributes): void { $this->attributes = $attributes; } @@ -140,10 +131,7 @@ public function getAttribute(string $name): mixed return $this->attributes[$name]; } - /** - * @return void - */ - public function setAttribute(string $name, mixed $value) + public function setAttribute(string $name, mixed $value): void { $this->attributes[$name] = $value; } diff --git a/Authentication/Token/NullToken.php b/Authentication/Token/NullToken.php index eabfe17b..9c2e4892 100644 --- a/Authentication/Token/NullToken.php +++ b/Authentication/Token/NullToken.php @@ -33,10 +33,7 @@ public function getUser(): ?UserInterface return null; } - /** - * @return never - */ - public function setUser(UserInterface $user) + public function setUser(UserInterface $user): never { throw new \BadMethodCallException('Cannot set user on a NullToken.'); } @@ -46,10 +43,7 @@ public function getUserIdentifier(): string return ''; } - /** - * @return void - */ - public function eraseCredentials() + public function eraseCredentials(): void { } @@ -58,10 +52,7 @@ public function getAttributes(): array return []; } - /** - * @return never - */ - public function setAttributes(array $attributes) + public function setAttributes(array $attributes): never { throw new \BadMethodCallException('Cannot set attributes of NullToken.'); } @@ -76,10 +67,7 @@ public function getAttribute(string $name): mixed return null; } - /** - * @return never - */ - public function setAttribute(string $name, mixed $value) + public function setAttribute(string $name, mixed $value): never { throw new \BadMethodCallException('Cannot add attribute to NullToken.'); } diff --git a/Authentication/Token/Storage/TokenStorage.php b/Authentication/Token/Storage/TokenStorage.php index 8acc31bc..42234f86 100644 --- a/Authentication/Token/Storage/TokenStorage.php +++ b/Authentication/Token/Storage/TokenStorage.php @@ -37,10 +37,7 @@ public function getToken(): ?TokenInterface return $this->token; } - /** - * @return void - */ - public function setToken(?TokenInterface $token) + public function setToken(?TokenInterface $token): void { if ($token) { // ensure any initializer is called @@ -56,10 +53,7 @@ public function setInitializer(?callable $initializer): void $this->initializer = null === $initializer ? null : $initializer(...); } - /** - * @return void - */ - public function reset() + public function reset(): void { $this->setToken(null); } diff --git a/Authentication/Token/Storage/TokenStorageInterface.php b/Authentication/Token/Storage/TokenStorageInterface.php index 5fdfa4e9..0f611ca4 100644 --- a/Authentication/Token/Storage/TokenStorageInterface.php +++ b/Authentication/Token/Storage/TokenStorageInterface.php @@ -29,8 +29,6 @@ public function getToken(): ?TokenInterface; * Sets the authentication token. * * @param TokenInterface|null $token A TokenInterface token, or null if no further authentication information should be stored - * - * @return void */ - public function setToken(?TokenInterface $token); + public function setToken(?TokenInterface $token): void; } diff --git a/Authentication/Token/TokenInterface.php b/Authentication/Token/TokenInterface.php index d9b80ae1..6d74add1 100644 --- a/Authentication/Token/TokenInterface.php +++ b/Authentication/Token/TokenInterface.php @@ -50,27 +50,21 @@ public function getUser(): ?UserInterface; /** * Sets the authenticated user in the token. * - * @return void - * * @throws \InvalidArgumentException */ - public function setUser(UserInterface $user); + public function setUser(UserInterface $user): void; /** * Removes sensitive information from the token. - * - * @return void */ - public function eraseCredentials(); + public function eraseCredentials(): void; public function getAttributes(): array; /** * @param array $attributes The token attributes - * - * @return void */ - public function setAttributes(array $attributes); + public function setAttributes(array $attributes): void; public function hasAttribute(string $name): bool; @@ -79,10 +73,7 @@ public function hasAttribute(string $name): bool; */ public function getAttribute(string $name): mixed; - /** - * @return void - */ - public function setAttribute(string $name, mixed $value); + public function setAttribute(string $name, mixed $value): void; /** * Returns all the necessary state of the object for serialization purposes. diff --git a/Authorization/Voter/RoleHierarchyVoter.php b/Authorization/Voter/RoleHierarchyVoter.php index c8db1485..3535ca11 100644 --- a/Authorization/Voter/RoleHierarchyVoter.php +++ b/Authorization/Voter/RoleHierarchyVoter.php @@ -31,10 +31,7 @@ public function __construct(RoleHierarchyInterface $roleHierarchy, string $prefi parent::__construct($prefix); } - /** - * @return array - */ - protected function extractRoles(TokenInterface $token) + protected function extractRoles(TokenInterface $token): array { return $this->roleHierarchy->getReachableRoleNames($token->getRoleNames()); } diff --git a/Authorization/Voter/RoleVoter.php b/Authorization/Voter/RoleVoter.php index 70dddcff..dbf50478 100644 --- a/Authorization/Voter/RoleVoter.php +++ b/Authorization/Voter/RoleVoter.php @@ -58,10 +58,7 @@ public function supportsType(string $subjectType): bool return true; } - /** - * @return array - */ - protected function extractRoles(TokenInterface $token) + protected function extractRoles(TokenInterface $token): array { return $token->getRoleNames(); } diff --git a/Event/AuthenticationEvent.php b/Event/AuthenticationEvent.php index 054dd957..6fca50d4 100644 --- a/Event/AuthenticationEvent.php +++ b/Event/AuthenticationEvent.php @@ -28,10 +28,7 @@ public function __construct(TokenInterface $token) $this->authenticationToken = $token; } - /** - * @return TokenInterface - */ - public function getAuthenticationToken() + public function getAuthenticationToken(): TokenInterface { return $this->authenticationToken; } diff --git a/Exception/AccessDeniedException.php b/Exception/AccessDeniedException.php index c95bae03..43df719e 100644 --- a/Exception/AccessDeniedException.php +++ b/Exception/AccessDeniedException.php @@ -34,10 +34,7 @@ public function getAttributes(): array return $this->attributes; } - /** - * @return void - */ - public function setAttributes(array|string $attributes) + public function setAttributes(array|string $attributes): void { $this->attributes = (array) $attributes; } @@ -47,10 +44,7 @@ public function getSubject(): mixed return $this->subject; } - /** - * @return void - */ - public function setSubject(mixed $subject) + public function setSubject(mixed $subject): void { $this->subject = $subject; } diff --git a/Exception/AccountStatusException.php b/Exception/AccountStatusException.php index 5b064929..c0176e08 100644 --- a/Exception/AccountStatusException.php +++ b/Exception/AccountStatusException.php @@ -32,10 +32,7 @@ public function getUser(): ?UserInterface return $this->user; } - /** - * @return void - */ - public function setUser(UserInterface $user) + public function setUser(UserInterface $user): void { $this->user = $user; } diff --git a/Exception/AuthenticationException.php b/Exception/AuthenticationException.php index adec6f87..548d4853 100644 --- a/Exception/AuthenticationException.php +++ b/Exception/AuthenticationException.php @@ -39,10 +39,7 @@ public function getToken(): ?TokenInterface return $this->token; } - /** - * @return void - */ - public function setToken(TokenInterface $token) + public function setToken(TokenInterface $token): void { $this->token = $token; } @@ -90,10 +87,8 @@ public function __unserialize(array $data): void /** * Message key to be used by the translation component. - * - * @return string */ - public function getMessageKey() + public function getMessageKey(): string { return 'An authentication exception occurred.'; } diff --git a/Exception/CustomUserMessageAccountStatusException.php b/Exception/CustomUserMessageAccountStatusException.php index 829a22cd..84b1c00a 100644 --- a/Exception/CustomUserMessageAccountStatusException.php +++ b/Exception/CustomUserMessageAccountStatusException.php @@ -38,10 +38,8 @@ public function __construct(string $message = '', array $messageData = [], int $ * * @param string $messageKey The message or message key * @param array $messageData Data to be passed into the translator - * - * @return void */ - public function setSafeMessage(string $messageKey, array $messageData = []) + public function setSafeMessage(string $messageKey, array $messageData = []): void { $this->messageKey = $messageKey; $this->messageData = $messageData; diff --git a/Exception/CustomUserMessageAuthenticationException.php b/Exception/CustomUserMessageAuthenticationException.php index 40de92d2..88c5338f 100644 --- a/Exception/CustomUserMessageAuthenticationException.php +++ b/Exception/CustomUserMessageAuthenticationException.php @@ -37,10 +37,8 @@ public function __construct(string $message = '', array $messageData = [], int $ * * @param string $messageKey The message or message key * @param array $messageData Data to be passed into the translator - * - * @return void */ - public function setSafeMessage(string $messageKey, array $messageData = []) + public function setSafeMessage(string $messageKey, array $messageData = []): void { $this->messageKey = $messageKey; $this->messageData = $messageData; diff --git a/Role/RoleHierarchy.php b/Role/RoleHierarchy.php index da094d2b..acc1c251 100644 --- a/Role/RoleHierarchy.php +++ b/Role/RoleHierarchy.php @@ -49,10 +49,7 @@ public function getReachableRoleNames(array $roles): array return array_values(array_unique($reachableRoles)); } - /** - * @return void - */ - protected function buildRoleMap() + protected function buildRoleMap(): void { $this->map = []; foreach ($this->hierarchy as $main => $roles) { diff --git a/User/InMemoryUserChecker.php b/User/InMemoryUserChecker.php index a493b00e..61367c2c 100644 --- a/User/InMemoryUserChecker.php +++ b/User/InMemoryUserChecker.php @@ -20,10 +20,7 @@ */ class InMemoryUserChecker implements UserCheckerInterface { - /** - * @return void - */ - public function checkPreAuth(UserInterface $user) + public function checkPreAuth(UserInterface $user): void { if (!$user instanceof InMemoryUser) { return; @@ -36,10 +33,7 @@ public function checkPreAuth(UserInterface $user) } } - /** - * @return void - */ - public function checkPostAuth(UserInterface $user) + public function checkPostAuth(UserInterface $user): void { } } diff --git a/User/InMemoryUserProvider.php b/User/InMemoryUserProvider.php index 13441bc7..ff5f3595 100644 --- a/User/InMemoryUserProvider.php +++ b/User/InMemoryUserProvider.php @@ -49,10 +49,8 @@ public function __construct(array $users = []) /** * Adds a new User to the provider. - * - * @return void */ - public function createUser(UserInterface $user) + public function createUser(UserInterface $user): void { if (!$user instanceof InMemoryUser) { throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_debug_type($user))); diff --git a/User/UserCheckerInterface.php b/User/UserCheckerInterface.php index 91f21c71..480ba7b5 100644 --- a/User/UserCheckerInterface.php +++ b/User/UserCheckerInterface.php @@ -26,18 +26,14 @@ interface UserCheckerInterface /** * Checks the user account before authentication. * - * @return void - * * @throws AccountStatusException */ - public function checkPreAuth(UserInterface $user); + public function checkPreAuth(UserInterface $user): void; /** * Checks the user account after authentication. * - * @return void - * * @throws AccountStatusException */ - public function checkPostAuth(UserInterface $user); + public function checkPostAuth(UserInterface $user): void; } diff --git a/User/UserInterface.php b/User/UserInterface.php index ef22340a..50f8fb0f 100644 --- a/User/UserInterface.php +++ b/User/UserInterface.php @@ -51,10 +51,8 @@ public function getRoles(): array; * * This is important if, at any given point, sensitive information like * the plain-text password is stored on this object. - * - * @return void */ - public function eraseCredentials(); + public function eraseCredentials(): void; /** * Returns the identifier for this user (e.g. username or email address). diff --git a/User/UserProviderInterface.php b/User/UserProviderInterface.php index ec90d413..0502bfd2 100644 --- a/User/UserProviderInterface.php +++ b/User/UserProviderInterface.php @@ -39,19 +39,15 @@ interface UserProviderInterface * object can just be merged into some internal array of users / identity * map. * - * @return UserInterface - * * @throws UnsupportedUserException if the user is not supported * @throws UserNotFoundException if the user is not found */ - public function refreshUser(UserInterface $user); + public function refreshUser(UserInterface $user): UserInterface; /** * Whether this provider supports the given user class. - * - * @return bool */ - public function supportsClass(string $class); + public function supportsClass(string $class): bool; /** * Loads the user for the given user identifier (e.g. username or email). diff --git a/Validator/Constraints/UserPasswordValidator.php b/Validator/Constraints/UserPasswordValidator.php index 41670b27..3d6c7637 100644 --- a/Validator/Constraints/UserPasswordValidator.php +++ b/Validator/Constraints/UserPasswordValidator.php @@ -31,10 +31,7 @@ public function __construct(TokenStorageInterface $tokenStorage, PasswordHasherF $this->hasherFactory = $hasherFactory; } - /** - * @return void - */ - public function validate(mixed $password, Constraint $constraint) + public function validate(mixed $password, Constraint $constraint): void { if (!$constraint instanceof UserPassword) { throw new UnexpectedTypeException($constraint, UserPassword::class); From ade1448d6adf5b3987614e797685ec43e0f113bd Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 5 Jul 2023 17:25:57 +0200 Subject: [PATCH 06/52] [Security] Revert native return types on TokenProviderInterface --- .../RememberMe/TokenProviderInterface.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/Authentication/RememberMe/TokenProviderInterface.php b/Authentication/RememberMe/TokenProviderInterface.php index 8be82c26..cf41e0c5 100644 --- a/Authentication/RememberMe/TokenProviderInterface.php +++ b/Authentication/RememberMe/TokenProviderInterface.php @@ -23,24 +23,32 @@ interface TokenProviderInterface /** * Loads the active token for the given series. * + * @return PersistentTokenInterface + * * @throws TokenNotFoundException if the token is not found */ - public function loadTokenBySeries(string $series): PersistentTokenInterface; + public function loadTokenBySeries(string $series); /** * Deletes all tokens belonging to series. + * + * @return void */ - public function deleteTokenBySeries(string $series): void; + public function deleteTokenBySeries(string $series); /** * Updates the token according to this data. * + * @return void + * * @throws TokenNotFoundException if the token is not found */ - public function updateToken(string $series, #[\SensitiveParameter] string $tokenValue, \DateTime $lastUsed): void; + public function updateToken(string $series, #[\SensitiveParameter] string $tokenValue, \DateTime $lastUsed); /** * Creates a new token. + * + * @return void */ - public function createNewToken(PersistentTokenInterface $token): void; + public function createNewToken(PersistentTokenInterface $token); } From 9e130f3a4eddc81e81b860c6f9601feff211553b Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Mon, 24 Jul 2023 16:03:38 +0200 Subject: [PATCH 07/52] [Validator] Remove Doctrine annotations support --- Validator/Constraints/UserPassword.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Validator/Constraints/UserPassword.php b/Validator/Constraints/UserPassword.php index b1a340c8..e7b7cd26 100644 --- a/Validator/Constraints/UserPassword.php +++ b/Validator/Constraints/UserPassword.php @@ -13,10 +13,6 @@ use Symfony\Component\Validator\Constraint; -/** - * @Annotation - * @Target({"PROPERTY", "METHOD", "ANNOTATION"}) - */ #[\Attribute(\Attribute::TARGET_PROPERTY | \Attribute::TARGET_METHOD | \Attribute::IS_REPEATABLE)] class UserPassword extends Constraint { From ee0949879d3b05b61c30e710e03f2354ccdd8a91 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 21 Jul 2023 15:36:26 +0200 Subject: [PATCH 08/52] Add types to public and protected properties --- Role/RoleHierarchy.php | 5 +++-- Validator/Constraints/UserPassword.php | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Role/RoleHierarchy.php b/Role/RoleHierarchy.php index acc1c251..15c5750d 100644 --- a/Role/RoleHierarchy.php +++ b/Role/RoleHierarchy.php @@ -18,9 +18,10 @@ */ class RoleHierarchy implements RoleHierarchyInterface { - private array $hierarchy; /** @var array> */ - protected $map; + protected array $map; + + private array $hierarchy; /** * @param array> $hierarchy diff --git a/Validator/Constraints/UserPassword.php b/Validator/Constraints/UserPassword.php index e7b7cd26..6f4024c0 100644 --- a/Validator/Constraints/UserPassword.php +++ b/Validator/Constraints/UserPassword.php @@ -22,8 +22,8 @@ class UserPassword extends Constraint self::INVALID_PASSWORD_ERROR => 'INVALID_PASSWORD_ERROR', ]; - public $message = 'This value should be the user\'s current password.'; - public $service = 'security.validator.user_password'; + public string $message = 'This value should be the user\'s current password.'; + public string $service = 'security.validator.user_password'; public function __construct(array $options = null, string $message = null, string $service = null, array $groups = null, mixed $payload = null) { From 10a13dd18d5cff64ff90e8c0afad9f9fa4895f48 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 5 Aug 2023 11:18:22 +0200 Subject: [PATCH 09/52] Add 2 recently missing return types --- Authentication/RememberMe/InMemoryTokenProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Authentication/RememberMe/InMemoryTokenProvider.php b/Authentication/RememberMe/InMemoryTokenProvider.php index 0aef6a2c..5a1f5865 100644 --- a/Authentication/RememberMe/InMemoryTokenProvider.php +++ b/Authentication/RememberMe/InMemoryTokenProvider.php @@ -31,7 +31,7 @@ public function loadTokenBySeries(string $series): PersistentTokenInterface return $this->tokens[$series]; } - public function updateToken(string $series, #[\SensitiveParameter] string $tokenValue, \DateTimeInterface $lastUsed) + public function updateToken(string $series, #[\SensitiveParameter] string $tokenValue, \DateTimeInterface $lastUsed): void { if (!isset($this->tokens[$series])) { throw new TokenNotFoundException('No token found.'); From 0aa85ec9a269942b2552ca080e9728d3cc140a85 Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Sun, 27 Aug 2023 15:53:12 +0200 Subject: [PATCH 10/52] Remove getUsername methods in tests --- Tests/Authentication/AuthenticationTrustResolverTest.php | 4 ---- Tests/Authentication/Token/Fixtures/CustomUser.php | 5 ----- 2 files changed, 9 deletions(-) diff --git a/Tests/Authentication/AuthenticationTrustResolverTest.php b/Tests/Authentication/AuthenticationTrustResolverTest.php index 02149ce3..3e0a8d50 100644 --- a/Tests/Authentication/AuthenticationTrustResolverTest.php +++ b/Tests/Authentication/AuthenticationTrustResolverTest.php @@ -115,10 +115,6 @@ public function setUser($user): void { } - public function getUsername(): string - { - } - public function getUserIdentifier(): string { } diff --git a/Tests/Authentication/Token/Fixtures/CustomUser.php b/Tests/Authentication/Token/Fixtures/CustomUser.php index 52fea7a3..99302032 100644 --- a/Tests/Authentication/Token/Fixtures/CustomUser.php +++ b/Tests/Authentication/Token/Fixtures/CustomUser.php @@ -17,11 +17,6 @@ public function __construct(string $username, array $roles) $this->roles = $roles; } - public function getUsername(): string - { - return $this->username; - } - public function getUserIdentifier(): string { return $this->username; From e0b8cbdcb2dea1a8a42a97414c74d759f6b7670b Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 18 Oct 2023 14:57:55 +0200 Subject: [PATCH 11/52] [7.0] Cleanup legacy code paths --- Authentication/RememberMe/TokenProviderInterface.php | 4 +--- CHANGELOG.md | 1 + 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Authentication/RememberMe/TokenProviderInterface.php b/Authentication/RememberMe/TokenProviderInterface.php index d2f0c8cb..bfe49015 100644 --- a/Authentication/RememberMe/TokenProviderInterface.php +++ b/Authentication/RememberMe/TokenProviderInterface.php @@ -39,13 +39,11 @@ public function deleteTokenBySeries(string $series); /** * Updates the token according to this data. * - * @param \DateTimeInterface $lastUsed Accepting only DateTime is deprecated since Symfony 6.4 - * * @return void * * @throws TokenNotFoundException if the token is not found */ - public function updateToken(string $series, #[\SensitiveParameter] string $tokenValue, \DateTime $lastUsed); + public function updateToken(string $series, #[\SensitiveParameter] string $tokenValue, \DateTimeInterface $lastUsed); /** * Creates a new token. diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a451ccc..47b4a210 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ CHANGELOG * Remove the `Security` class, use `Symfony\Bundle\SecurityBundle\Security` instead * Require explicit argument when calling `TokenStorage::setToken()` + * Change argument `$lastUsed` of `TokenProviderInterface::updateToken()` to accept `DateTimeInterface` 6.4 --- From 7363e581fad8b0037ade4b0c803af7d11fd5066f Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Mon, 20 Nov 2023 22:20:58 +0100 Subject: [PATCH 12/52] [Security] remove conflict with symfony/security-guard --- composer.json | 1 - 1 file changed, 1 deletion(-) diff --git a/composer.json b/composer.json index 0bc2c140..51323362 100644 --- a/composer.json +++ b/composer.json @@ -37,7 +37,6 @@ "conflict": { "symfony/event-dispatcher": "<6.4", "symfony/http-foundation": "<6.4", - "symfony/security-guard": "<6.4", "symfony/ldap": "<6.4", "symfony/validator": "<6.4" }, From 2ba040de8e6d93e07edc7307dc75b42e06137405 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 30 Nov 2023 12:04:23 +0100 Subject: [PATCH 13/52] Update sponsors of Symfony 7.0: Shopware, Sulu and Les-Tilleuls + Sensiolabs for Messenger & SymfonyCasts for Security components --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 48ffb0e5..5bb87c3c 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ if (!$accessDecisionManager->decide($token, ['ROLE_ADMIN'])) { Sponsor ------- -The Security component for Symfony 6.4 is [backed][1] by [SymfonyCasts][2]. +The Security component for Symfony 7.0 is [backed][1] by [SymfonyCasts][2]. Learn Symfony faster by watching real projects being built and actively coding along with them. SymfonyCasts bridges that learning gap, bringing you video From aedbe2a4fe792f6a44b965b9f0e461e81ac51d25 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Fri, 8 Dec 2023 15:23:08 +0100 Subject: [PATCH 14/52] Fx README files --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 5bb87c3c..b7068290 100644 --- a/README.md +++ b/README.md @@ -8,8 +8,8 @@ so called user providers that hold the users credentials. Getting Started --------------- -``` -$ composer require symfony/security-core +```bash +composer require symfony/security-core ``` ```php From bb53c52b1063ac9bc850ff58d6b58b543766cdc9 Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Thu, 14 Dec 2023 11:03:37 +0100 Subject: [PATCH 15/52] Set `strict` parameter of `in_array` to true where possible --- Tests/Authorization/TraceableAccessDecisionManagerTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/Authorization/TraceableAccessDecisionManagerTest.php b/Tests/Authorization/TraceableAccessDecisionManagerTest.php index cefe8dbc..c14ee1fa 100644 --- a/Tests/Authorization/TraceableAccessDecisionManagerTest.php +++ b/Tests/Authorization/TraceableAccessDecisionManagerTest.php @@ -197,7 +197,7 @@ public function testAccessDecisionManagerCalledByVoter() ->expects($this->any()) ->method('vote') ->willReturnCallback(function (TokenInterface $token, $subject, array $attributes) use ($sut, $voter1) { - $vote = \in_array('attr1', $attributes) ? VoterInterface::ACCESS_GRANTED : VoterInterface::ACCESS_ABSTAIN; + $vote = \in_array('attr1', $attributes, true) ? VoterInterface::ACCESS_GRANTED : VoterInterface::ACCESS_ABSTAIN; $sut->addVoterVote($voter1, $attributes, $vote); return $vote; @@ -207,7 +207,7 @@ public function testAccessDecisionManagerCalledByVoter() ->expects($this->any()) ->method('vote') ->willReturnCallback(function (TokenInterface $token, $subject, array $attributes) use ($sut, $voter2) { - if (\in_array('attr2', $attributes)) { + if (\in_array('attr2', $attributes, true)) { $vote = null == $subject ? VoterInterface::ACCESS_GRANTED : VoterInterface::ACCESS_DENIED; } else { $vote = VoterInterface::ACCESS_ABSTAIN; @@ -222,7 +222,7 @@ public function testAccessDecisionManagerCalledByVoter() ->expects($this->any()) ->method('vote') ->willReturnCallback(function (TokenInterface $token, $subject, array $attributes) use ($sut, $voter3) { - if (\in_array('attr2', $attributes) && $subject) { + if (\in_array('attr2', $attributes, true) && $subject) { $vote = $sut->decide($token, $attributes) ? VoterInterface::ACCESS_GRANTED : VoterInterface::ACCESS_DENIED; } else { $vote = VoterInterface::ACCESS_ABSTAIN; From afa665fc5657918fbbec885b2ffdcc96e28c1333 Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Mon, 18 Dec 2023 08:46:12 +0100 Subject: [PATCH 16/52] Code updates --- Authorization/Voter/RoleVoter.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Authorization/Voter/RoleVoter.php b/Authorization/Voter/RoleVoter.php index dbf50478..76de3a32 100644 --- a/Authorization/Voter/RoleVoter.php +++ b/Authorization/Voter/RoleVoter.php @@ -38,10 +38,8 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes): } $result = VoterInterface::ACCESS_DENIED; - foreach ($roles as $role) { - if ($attribute === $role) { - return VoterInterface::ACCESS_GRANTED; - } + if (\in_array($attribute, $roles, true)) { + return VoterInterface::ACCESS_GRANTED; } } From e4f316c4abe3d05aab878086b82162e739b2935a Mon Sep 17 00:00:00 2001 From: Oskar Stark Date: Wed, 1 Nov 2023 09:14:07 +0100 Subject: [PATCH 17/52] [Tests] Streamline --- .../RememberMe/InMemoryTokenProviderTest.php | 7 ++++--- Tests/Authorization/Voter/VoterTest.php | 3 +-- Tests/User/ChainUserProviderTest.php | 8 ++++++-- Tests/User/InMemoryUserCheckerTest.php | 3 +-- Tests/User/InMemoryUserProviderTest.php | 7 ++++--- .../Constraints/UserPasswordValidatorTestCase.php | 3 ++- 6 files changed, 18 insertions(+), 13 deletions(-) diff --git a/Tests/Authentication/RememberMe/InMemoryTokenProviderTest.php b/Tests/Authentication/RememberMe/InMemoryTokenProviderTest.php index 45fd046a..6fc2ab15 100644 --- a/Tests/Authentication/RememberMe/InMemoryTokenProviderTest.php +++ b/Tests/Authentication/RememberMe/InMemoryTokenProviderTest.php @@ -31,8 +31,7 @@ public function testCreateNewToken() public function testLoadTokenBySeriesThrowsNotFoundException() { $this->expectException(TokenNotFoundException::class); - $provider = new InMemoryTokenProvider(); - $provider->loadTokenBySeries('foo'); + (new InMemoryTokenProvider())->loadTokenBySeries('foo'); } public function testUpdateToken() @@ -50,12 +49,14 @@ public function testUpdateToken() public function testDeleteToken() { - $this->expectException(TokenNotFoundException::class); $provider = new InMemoryTokenProvider(); $token = new PersistentToken('foo', 'foo', 'foo', 'foo', new \DateTimeImmutable()); $provider->createNewToken($token); $provider->deleteTokenBySeries('foo'); + + $this->expectException(TokenNotFoundException::class); + $provider->loadTokenBySeries('foo'); } } diff --git a/Tests/Authorization/Voter/VoterTest.php b/Tests/Authorization/Voter/VoterTest.php index 80c3f4a0..5636340e 100644 --- a/Tests/Authorization/Voter/VoterTest.php +++ b/Tests/Authorization/Voter/VoterTest.php @@ -67,8 +67,7 @@ public function testVoteWithTypeError() { $this->expectException(\TypeError::class); $this->expectExceptionMessage('Should error'); - $voter = new TypeErrorVoterTest_Voter(); - $voter->vote($this->token, new \stdClass(), ['EDIT']); + (new TypeErrorVoterTest_Voter())->vote($this->token, new \stdClass(), ['EDIT']); } } diff --git a/Tests/User/ChainUserProviderTest.php b/Tests/User/ChainUserProviderTest.php index 09227752..90111561 100644 --- a/Tests/User/ChainUserProviderTest.php +++ b/Tests/User/ChainUserProviderTest.php @@ -47,7 +47,6 @@ public function testLoadUserByIdentifier() public function testLoadUserByIdentifierThrowsUserNotFoundException() { - $this->expectException(UserNotFoundException::class); $provider1 = $this->createMock(InMemoryUserProvider::class); $provider1 ->expects($this->once()) @@ -65,6 +64,9 @@ public function testLoadUserByIdentifierThrowsUserNotFoundException() ; $provider = new ChainUserProvider([$provider1, $provider2]); + + $this->expectException(UserNotFoundException::class); + $provider->loadUserByIdentifier('foo'); } @@ -141,7 +143,6 @@ public function testRefreshUserAgain() public function testRefreshUserThrowsUnsupportedUserException() { - $this->expectException(UnsupportedUserException::class); $provider1 = $this->createMock(InMemoryUserProvider::class); $provider1 ->expects($this->once()) @@ -169,6 +170,9 @@ public function testRefreshUserThrowsUnsupportedUserException() ; $provider = new ChainUserProvider([$provider1, $provider2]); + + $this->expectException(UnsupportedUserException::class); + $provider->refreshUser($this->createMock(UserInterface::class)); } diff --git a/Tests/User/InMemoryUserCheckerTest.php b/Tests/User/InMemoryUserCheckerTest.php index 8b01e5f0..25107723 100644 --- a/Tests/User/InMemoryUserCheckerTest.php +++ b/Tests/User/InMemoryUserCheckerTest.php @@ -35,7 +35,6 @@ public function testCheckPostAuthPass() public function testCheckPreAuthDisabled() { $this->expectException(DisabledException::class); - $checker = new InMemoryUserChecker(); - $checker->checkPreAuth(new InMemoryUser('John', 'password', [], false)); + (new InMemoryUserChecker())->checkPreAuth(new InMemoryUser('John', 'password', [], false)); } } diff --git a/Tests/User/InMemoryUserProviderTest.php b/Tests/User/InMemoryUserProviderTest.php index 1a843e4e..98afb3b4 100644 --- a/Tests/User/InMemoryUserProviderTest.php +++ b/Tests/User/InMemoryUserProviderTest.php @@ -62,16 +62,17 @@ public function testCreateUser() public function testCreateUserAlreadyExist() { - $this->expectException(\LogicException::class); $provider = new InMemoryUserProvider(); $provider->createUser(new InMemoryUser('fabien', 'foo')); + + $this->expectException(\LogicException::class); + $provider->createUser(new InMemoryUser('fabien', 'foo')); } public function testLoadUserByIdentifierDoesNotExist() { $this->expectException(UserNotFoundException::class); - $provider = new InMemoryUserProvider(); - $provider->loadUserByIdentifier('fabien'); + (new InMemoryUserProvider())->loadUserByIdentifier('fabien'); } } diff --git a/Tests/Validator/Constraints/UserPasswordValidatorTestCase.php b/Tests/Validator/Constraints/UserPasswordValidatorTestCase.php index ccf556a0..c78f6b5f 100644 --- a/Tests/Validator/Constraints/UserPasswordValidatorTestCase.php +++ b/Tests/Validator/Constraints/UserPasswordValidatorTestCase.php @@ -113,13 +113,14 @@ public static function emptyPasswordData() public function testUserIsNotValid() { - $this->expectException(ConstraintDefinitionException::class); $user = new \stdClass(); $this->tokenStorage = $this->createTokenStorage($user); $this->validator = $this->createValidator(); $this->validator->initialize($this->context); + $this->expectException(ConstraintDefinitionException::class); + $this->validator->validate('secret', new UserPassword()); } From 891b5bdad89d7331747e3501c0dee07000f6906b Mon Sep 17 00:00:00 2001 From: Dariusz Ruminski Date: Tue, 2 Jan 2024 15:49:33 +0100 Subject: [PATCH 18/52] CS: trailing commas --- User/OidcUser.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/User/OidcUser.php b/User/OidcUser.php index 490a3ec2..bcce363f 100644 --- a/User/OidcUser.php +++ b/User/OidcUser.php @@ -45,7 +45,7 @@ public function __construct( private ?\DateTimeInterface $updatedAt = null, // Additional Claims (https://openid.net/specs/openid-connect-core-1_0.html#AdditionalClaims) - ...$additionalClaims + ...$additionalClaims, ) { if (null === $sub || '' === $sub) { throw new \InvalidArgumentException('The "sub" claim cannot be empty.'); From 918585786b2ebff1d325a1872a12c0d1e3589f90 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Thu, 11 Jan 2024 11:01:49 +0100 Subject: [PATCH 19/52] do not mock the RequestStack class --- .../Storage/UsageTrackingTokenStorageTest.php | 37 ++++++------------- composer.json | 2 + 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/Tests/Authentication/Token/Storage/UsageTrackingTokenStorageTest.php b/Tests/Authentication/Token/Storage/UsageTrackingTokenStorageTest.php index 3d7c7904..72e1665a 100644 --- a/Tests/Authentication/Token/Storage/UsageTrackingTokenStorageTest.php +++ b/Tests/Authentication/Token/Storage/UsageTrackingTokenStorageTest.php @@ -13,9 +13,10 @@ use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; +use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; -use Symfony\Component\HttpFoundation\Session\SessionInterface; +use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage; @@ -25,28 +26,14 @@ class UsageTrackingTokenStorageTest extends TestCase { public function testGetSetToken() { - $sessionAccess = 0; - $sessionLocator = new class(['request_stack' => function () use (&$sessionAccess) { - $session = $this->createMock(SessionInterface::class); - - $request = new Request(); - $request->setSession($session); - $requestStack = $this->getMockBuilder(RequestStack::class)->onlyMethods(['getSession'])->getMock(); - $requestStack->push($request); - $requestStack->expects($this->any())->method('getSession')->willReturnCallback(function () use ($session, &$sessionAccess) { - ++$sessionAccess; - - $session->expects($this->once()) - ->method('getMetadataBag'); - - return $session; - }); - - return $requestStack; - }]) implements ContainerInterface { - use ServiceLocatorTrait; - }; $tokenStorage = new TokenStorage(); + $session = new Session(); + $request = new Request(); + $request->setSession($session); + $requestStack = new RequestStack(); + $requestStack->push($request); + $sessionLocator = new ContainerBuilder(); + $sessionLocator->set('request_stack', $requestStack); $trackingStorage = new UsageTrackingTokenStorage($tokenStorage, $sessionLocator); $this->assertNull($trackingStorage->getToken()); @@ -55,15 +42,15 @@ public function testGetSetToken() $trackingStorage->setToken($token); $this->assertSame($token, $trackingStorage->getToken()); $this->assertSame($token, $tokenStorage->getToken()); - $this->assertSame(0, $sessionAccess); + $this->assertSame(0, $session->getUsageIndex()); $trackingStorage->enableUsageTracking(); $this->assertSame($token, $trackingStorage->getToken()); - $this->assertSame(1, $sessionAccess); + $this->assertSame(1, $session->getUsageIndex()); $trackingStorage->disableUsageTracking(); $this->assertSame($token, $trackingStorage->getToken()); - $this->assertSame(1, $sessionAccess); + $this->assertSame(1, $session->getUsageIndex()); } public function testWithoutMainRequest() diff --git a/composer.json b/composer.json index bb6c4d50..a923768b 100644 --- a/composer.json +++ b/composer.json @@ -25,6 +25,7 @@ "psr/container": "^1.1|^2.0", "psr/cache": "^1.0|^2.0|^3.0", "symfony/cache": "^6.4|^7.0", + "symfony/dependency-injection": "^6.4|^7.0", "symfony/event-dispatcher": "^6.4|^7.0", "symfony/expression-language": "^6.4|^7.0", "symfony/http-foundation": "^6.4|^7.0", @@ -35,6 +36,7 @@ "psr/log": "^1|^2|^3" }, "conflict": { + "symfony/dependency-injection": "<6.4", "symfony/event-dispatcher": "<6.4", "symfony/http-foundation": "<6.4", "symfony/ldap": "<6.4", From 220301f8a7cdb0b11873aa242644a6f98f893fbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Ostroluck=C3=BD?= Date: Sun, 31 Mar 2024 15:15:18 +0200 Subject: [PATCH 20/52] Remove unnecessary empty usages --- Authentication/RememberMe/PersistentToken.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Authentication/RememberMe/PersistentToken.php b/Authentication/RememberMe/PersistentToken.php index f473ccb7..c1f1e1a7 100644 --- a/Authentication/RememberMe/PersistentToken.php +++ b/Authentication/RememberMe/PersistentToken.php @@ -26,16 +26,16 @@ final class PersistentToken implements PersistentTokenInterface public function __construct(string $class, string $userIdentifier, string $series, #[\SensitiveParameter] string $tokenValue, \DateTimeInterface $lastUsed) { - if (empty($class)) { + if (!$class) { throw new \InvalidArgumentException('$class must not be empty.'); } if ('' === $userIdentifier) { throw new \InvalidArgumentException('$userIdentifier must not be empty.'); } - if (empty($series)) { + if (!$series) { throw new \InvalidArgumentException('$series must not be empty.'); } - if (empty($tokenValue)) { + if (!$tokenValue) { throw new \InvalidArgumentException('$tokenValue must not be empty.'); } From 74717b37d89469ce9206ea830e3effd2c1e75afe Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 2 May 2024 13:36:26 +0200 Subject: [PATCH 21/52] [Security] SymfonyCasts is backing the security components for v7.1, thanks to them! \o/ --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b7068290..fc50dcc6 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ if (!$accessDecisionManager->decide($token, ['ROLE_ADMIN'])) { Sponsor ------- -The Security component for Symfony 7.0 is [backed][1] by [SymfonyCasts][2]. +The Security component for Symfony 7.1 is [backed][1] by [SymfonyCasts][2]. Learn Symfony faster by watching real projects being built and actively coding along with them. SymfonyCasts bridges that learning gap, bringing you video From 74ebf9b36c60ec3881c7dcdd3d0ee17cfca0a84b Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 21 May 2024 11:22:57 +0200 Subject: [PATCH 22/52] [Security] Deprecate argument $secret of RememberMeToken and RememberMeAuthenticator --- Authentication/Token/RememberMeToken.php | 20 +++++++++++-------- CHANGELOG.md | 4 ++++ .../AuthenticationTrustResolverTest.php | 2 +- .../Token/RememberMeTokenTest.php | 18 ++++++++--------- .../Authorization/ExpressionLanguageTest.php | 2 +- .../Voter/AuthenticatedVoterTest.php | 2 +- composer.json | 1 + 7 files changed, 29 insertions(+), 20 deletions(-) diff --git a/Authentication/Token/RememberMeToken.php b/Authentication/Token/RememberMeToken.php index ad218f1b..643c40d4 100644 --- a/Authentication/Token/RememberMeToken.php +++ b/Authentication/Token/RememberMeToken.php @@ -21,20 +21,19 @@ */ class RememberMeToken extends AbstractToken { - private string $secret; + private ?string $secret = null; private string $firewallName; /** - * @param string $secret A secret used to make sure the token is created by the app and not by a malicious client - * * @throws \InvalidArgumentException */ - public function __construct(UserInterface $user, string $firewallName, #[\SensitiveParameter] string $secret) + public function __construct(UserInterface $user, string $firewallName) { parent::__construct($user->getRoles()); - if (!$secret) { - throw new InvalidArgumentException('A non-empty secret is required.'); + if (\func_num_args() > 2) { + trigger_deprecation('symfony/security-core', '7.2', 'The "$secret" argument of "%s()" is deprecated.', __METHOD__); + $this->secret = func_get_arg(2); } if (!$firewallName) { @@ -42,7 +41,6 @@ public function __construct(UserInterface $user, string $firewallName, #[\Sensit } $this->firewallName = $firewallName; - $this->secret = $secret; $this->setUser($user); } @@ -52,13 +50,19 @@ public function getFirewallName(): string return $this->firewallName; } + /** + * @deprecated since Symfony 7.2 + */ public function getSecret(): string { - return $this->secret; + trigger_deprecation('symfony/security-core', '7.2', 'The "%s()" method is deprecated.', __METHOD__); + + return $this->secret ??= base64_encode(random_bytes(8)); } public function __serialize(): array { + // $this->firewallName should be kept at index 1 for compatibility with payloads generated before Symfony 8 return [$this->secret, $this->firewallName, parent::__serialize()]; } diff --git a/CHANGELOG.md b/CHANGELOG.md index 47b4a210..208f0d48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ CHANGELOG ========= +7.2 +--- + + * Deprecate argument `$secret` of `RememberMeToken` 7.0 --- diff --git a/Tests/Authentication/AuthenticationTrustResolverTest.php b/Tests/Authentication/AuthenticationTrustResolverTest.php index 3e0a8d50..fc559983 100644 --- a/Tests/Authentication/AuthenticationTrustResolverTest.php +++ b/Tests/Authentication/AuthenticationTrustResolverTest.php @@ -72,7 +72,7 @@ protected function getRememberMeToken() { $user = new InMemoryUser('wouter', '', ['ROLE_USER']); - return new RememberMeToken($user, 'main', 'secret'); + return new RememberMeToken($user, 'main'); } } diff --git a/Tests/Authentication/Token/RememberMeTokenTest.php b/Tests/Authentication/Token/RememberMeTokenTest.php index a63d481b..b0cdbaf1 100644 --- a/Tests/Authentication/Token/RememberMeTokenTest.php +++ b/Tests/Authentication/Token/RememberMeTokenTest.php @@ -20,22 +20,22 @@ class RememberMeTokenTest extends TestCase public function testConstructor() { $user = $this->getUser(); - $token = new RememberMeToken($user, 'fookey', 'foo'); + $token = new RememberMeToken($user, 'fookey'); $this->assertEquals('fookey', $token->getFirewallName()); - $this->assertEquals('foo', $token->getSecret()); $this->assertEquals(['ROLE_FOO'], $token->getRoleNames()); $this->assertSame($user, $token->getUser()); } - public function testConstructorSecretCannotBeEmptyString() + /** + * @group legacy + */ + public function testSecret() { - $this->expectException(\InvalidArgumentException::class); - new RememberMeToken( - $this->getUser(), - '', - '' - ); + $user = $this->getUser(); + $token = new RememberMeToken($user, 'fookey', 'foo'); + + $this->assertEquals('foo', $token->getSecret()); } protected function getUser($roles = ['ROLE_FOO']) diff --git a/Tests/Authorization/ExpressionLanguageTest.php b/Tests/Authorization/ExpressionLanguageTest.php index 8cc4810a..1a4db41e 100644 --- a/Tests/Authorization/ExpressionLanguageTest.php +++ b/Tests/Authorization/ExpressionLanguageTest.php @@ -50,7 +50,7 @@ public static function provider() $user = new InMemoryUser('username', 'password', $roles); $noToken = null; - $rememberMeToken = new RememberMeToken($user, 'firewall-name', 'firewall'); + $rememberMeToken = new RememberMeToken($user, 'firewall-name'); $usernamePasswordToken = new UsernamePasswordToken($user, 'firewall-name', $roles); return [ diff --git a/Tests/Authorization/Voter/AuthenticatedVoterTest.php b/Tests/Authorization/Voter/AuthenticatedVoterTest.php index 88544c08..3a3b9d4e 100644 --- a/Tests/Authorization/Voter/AuthenticatedVoterTest.php +++ b/Tests/Authorization/Voter/AuthenticatedVoterTest.php @@ -101,7 +101,7 @@ public function getCredentials() } if ('remembered' === $authenticated) { - return new RememberMeToken($user, 'foo', 'bar'); + return new RememberMeToken($user, 'foo'); } if ('impersonated' === $authenticated) { diff --git a/composer.json b/composer.json index a923768b..0aaff1e3 100644 --- a/composer.json +++ b/composer.json @@ -17,6 +17,7 @@ ], "require": { "php": ">=8.2", + "symfony/deprecation-contracts": "^2.5|^3", "symfony/event-dispatcher-contracts": "^2.5|^3", "symfony/service-contracts": "^2.5|^3", "symfony/password-hasher": "^6.4|^7.0" From 8acc2fbaa06386de128a2a843a30728660c105c4 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Mon, 3 Jun 2024 09:09:05 +0200 Subject: [PATCH 23/52] use constructor property promotion --- .../RememberMe/CacheTokenVerifier.php | 14 +++++-------- Authentication/RememberMe/PersistentToken.php | 17 +++++++-------- .../Token/PreAuthenticatedToken.php | 10 ++++----- Authentication/Token/RememberMeToken.php | 9 ++++---- .../Storage/UsageTrackingTokenStorage.php | 10 ++++----- Authentication/Token/SwitchUserToken.php | 11 ++++++---- .../Token/UsernamePasswordToken.php | 10 ++++----- Authorization/AccessDecisionManager.php | 8 +++---- .../Strategy/AffirmativeStrategy.php | 8 +++---- Authorization/Strategy/ConsensusStrategy.php | 11 ++++------ Authorization/Strategy/PriorityStrategy.php | 8 +++---- Authorization/Strategy/UnanimousStrategy.php | 8 +++---- .../TraceableAccessDecisionManager.php | 8 +++---- Authorization/Voter/AuthenticatedVoter.php | 8 +++---- Authorization/Voter/ExpressionVoter.php | 17 ++++++--------- Authorization/Voter/RoleHierarchyVoter.php | 10 ++++----- Authorization/Voter/RoleVoter.php | 8 +++---- Authorization/Voter/TraceableVoter.php | 11 ++++------ Event/AuthenticationEvent.php | 10 ++++----- Event/VoteEvent.php | 17 ++++++--------- Exception/LazyResponseException.php | 8 +++---- ...nyLoginAttemptsAuthenticationException.php | 8 +++---- Role/RoleHierarchy.php | 9 +++----- Signature/ExpiredSignatureStorage.php | 11 ++++------ Signature/SignatureHasher.php | 21 +++++++------------ Test/AccessDecisionStrategyTestCase.php | 8 +++---- User/ChainUserProvider.php | 8 +++---- User/InMemoryUser.php | 14 ++++++------- .../Constraints/UserPasswordValidator.php | 11 ++++------ 29 files changed, 123 insertions(+), 188 deletions(-) diff --git a/Authentication/RememberMe/CacheTokenVerifier.php b/Authentication/RememberMe/CacheTokenVerifier.php index e4f1362a..3930ac8d 100644 --- a/Authentication/RememberMe/CacheTokenVerifier.php +++ b/Authentication/RememberMe/CacheTokenVerifier.php @@ -18,21 +18,17 @@ */ class CacheTokenVerifier implements TokenVerifierInterface { - private CacheItemPoolInterface $cache; - private int $outdatedTokenTtl; - private string $cacheKeyPrefix; - /** * @param int $outdatedTokenTtl How long the outdated token should still be considered valid. Defaults * to 60, which matches how often the PersistentRememberMeHandler will at * most refresh tokens. Increasing to more than that is not recommended, * but you may use a lower value. */ - public function __construct(CacheItemPoolInterface $cache, int $outdatedTokenTtl = 60, string $cacheKeyPrefix = 'rememberme-stale-') - { - $this->cache = $cache; - $this->outdatedTokenTtl = $outdatedTokenTtl; - $this->cacheKeyPrefix = $cacheKeyPrefix; + public function __construct( + private CacheItemPoolInterface $cache, + private int $outdatedTokenTtl = 60, + private string $cacheKeyPrefix = 'rememberme-stale-', + ) { } public function verifyToken(PersistentTokenInterface $token, #[\SensitiveParameter] string $tokenValue): bool diff --git a/Authentication/RememberMe/PersistentToken.php b/Authentication/RememberMe/PersistentToken.php index c1f1e1a7..0f391c23 100644 --- a/Authentication/RememberMe/PersistentToken.php +++ b/Authentication/RememberMe/PersistentToken.php @@ -18,14 +18,15 @@ */ final class PersistentToken implements PersistentTokenInterface { - private string $class; - private string $userIdentifier; - private string $series; - private string $tokenValue; private \DateTimeImmutable $lastUsed; - public function __construct(string $class, string $userIdentifier, string $series, #[\SensitiveParameter] string $tokenValue, \DateTimeInterface $lastUsed) - { + public function __construct( + private string $class, + private string $userIdentifier, + private string $series, + #[\SensitiveParameter] private string $tokenValue, + \DateTimeInterface $lastUsed, + ) { if (!$class) { throw new \InvalidArgumentException('$class must not be empty.'); } @@ -39,10 +40,6 @@ public function __construct(string $class, string $userIdentifier, string $serie throw new \InvalidArgumentException('$tokenValue must not be empty.'); } - $this->class = $class; - $this->userIdentifier = $userIdentifier; - $this->series = $series; - $this->tokenValue = $tokenValue; $this->lastUsed = \DateTimeImmutable::createFromInterface($lastUsed); } diff --git a/Authentication/Token/PreAuthenticatedToken.php b/Authentication/Token/PreAuthenticatedToken.php index a216d4c1..5c092404 100644 --- a/Authentication/Token/PreAuthenticatedToken.php +++ b/Authentication/Token/PreAuthenticatedToken.php @@ -20,13 +20,14 @@ */ class PreAuthenticatedToken extends AbstractToken { - private string $firewallName; - /** * @param string[] $roles */ - public function __construct(UserInterface $user, string $firewallName, array $roles = []) - { + public function __construct( + UserInterface $user, + private string $firewallName, + array $roles = [], + ) { parent::__construct($roles); if ('' === $firewallName) { @@ -34,7 +35,6 @@ public function __construct(UserInterface $user, string $firewallName, array $ro } $this->setUser($user); - $this->firewallName = $firewallName; } public function getFirewallName(): string diff --git a/Authentication/Token/RememberMeToken.php b/Authentication/Token/RememberMeToken.php index 643c40d4..dfbe20ec 100644 --- a/Authentication/Token/RememberMeToken.php +++ b/Authentication/Token/RememberMeToken.php @@ -22,13 +22,14 @@ class RememberMeToken extends AbstractToken { private ?string $secret = null; - private string $firewallName; /** * @throws \InvalidArgumentException */ - public function __construct(UserInterface $user, string $firewallName) - { + public function __construct( + UserInterface $user, + private string $firewallName, + ) { parent::__construct($user->getRoles()); if (\func_num_args() > 2) { @@ -40,8 +41,6 @@ public function __construct(UserInterface $user, string $firewallName) throw new InvalidArgumentException('$firewallName must not be empty.'); } - $this->firewallName = $firewallName; - $this->setUser($user); } diff --git a/Authentication/Token/Storage/UsageTrackingTokenStorage.php b/Authentication/Token/Storage/UsageTrackingTokenStorage.php index 8a4069e7..4255491d 100644 --- a/Authentication/Token/Storage/UsageTrackingTokenStorage.php +++ b/Authentication/Token/Storage/UsageTrackingTokenStorage.php @@ -24,14 +24,12 @@ */ final class UsageTrackingTokenStorage implements TokenStorageInterface, ServiceSubscriberInterface { - private TokenStorageInterface $storage; - private ContainerInterface $container; private bool $enableUsageTracking = false; - public function __construct(TokenStorageInterface $storage, ContainerInterface $container) - { - $this->storage = $storage; - $this->container = $container; + public function __construct( + private TokenStorageInterface $storage, + private ContainerInterface $container, + ) { } public function getToken(): ?TokenInterface diff --git a/Authentication/Token/SwitchUserToken.php b/Authentication/Token/SwitchUserToken.php index fb632a61..c4e69766 100644 --- a/Authentication/Token/SwitchUserToken.php +++ b/Authentication/Token/SwitchUserToken.php @@ -20,7 +20,6 @@ */ class SwitchUserToken extends UsernamePasswordToken { - private TokenInterface $originalToken; private ?string $originatedFromUri = null; /** @@ -29,11 +28,15 @@ class SwitchUserToken extends UsernamePasswordToken * * @throws \InvalidArgumentException */ - public function __construct(UserInterface $user, string $firewallName, array $roles, TokenInterface $originalToken, ?string $originatedFromUri = null) - { + public function __construct( + UserInterface $user, + string $firewallName, + array $roles, + private TokenInterface $originalToken, + ?string $originatedFromUri = null, + ) { parent::__construct($user, $firewallName, $roles); - $this->originalToken = $originalToken; $this->originatedFromUri = $originatedFromUri; } diff --git a/Authentication/Token/UsernamePasswordToken.php b/Authentication/Token/UsernamePasswordToken.php index 74e24a21..40beb003 100644 --- a/Authentication/Token/UsernamePasswordToken.php +++ b/Authentication/Token/UsernamePasswordToken.php @@ -20,10 +20,11 @@ */ class UsernamePasswordToken extends AbstractToken { - private string $firewallName; - - public function __construct(UserInterface $user, string $firewallName, array $roles = []) - { + public function __construct( + UserInterface $user, + private string $firewallName, + array $roles = [], + ) { parent::__construct($roles); if ('' === $firewallName) { @@ -31,7 +32,6 @@ public function __construct(UserInterface $user, string $firewallName, array $ro } $this->setUser($user); - $this->firewallName = $firewallName; } public function getFirewallName(): string diff --git a/Authorization/AccessDecisionManager.php b/Authorization/AccessDecisionManager.php index 4a56f943..10969acb 100644 --- a/Authorization/AccessDecisionManager.php +++ b/Authorization/AccessDecisionManager.php @@ -32,7 +32,6 @@ final class AccessDecisionManager implements AccessDecisionManagerInterface VoterInterface::ACCESS_ABSTAIN => true, ]; - private iterable $voters; private array $votersCacheAttributes = []; private array $votersCacheObject = []; private AccessDecisionStrategyInterface $strategy; @@ -40,9 +39,10 @@ final class AccessDecisionManager implements AccessDecisionManagerInterface /** * @param iterable $voters An array or an iterator of VoterInterface instances */ - public function __construct(iterable $voters = [], ?AccessDecisionStrategyInterface $strategy = null) - { - $this->voters = $voters; + public function __construct( + private iterable $voters = [], + ?AccessDecisionStrategyInterface $strategy = null, + ) { $this->strategy = $strategy ?? new AffirmativeStrategy(); } diff --git a/Authorization/Strategy/AffirmativeStrategy.php b/Authorization/Strategy/AffirmativeStrategy.php index ecd74b20..fb316fd3 100644 --- a/Authorization/Strategy/AffirmativeStrategy.php +++ b/Authorization/Strategy/AffirmativeStrategy.php @@ -24,11 +24,9 @@ */ final class AffirmativeStrategy implements AccessDecisionStrategyInterface, \Stringable { - private bool $allowIfAllAbstainDecisions; - - public function __construct(bool $allowIfAllAbstainDecisions = false) - { - $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; + public function __construct( + private bool $allowIfAllAbstainDecisions = false, + ) { } public function decide(\Traversable $results): bool diff --git a/Authorization/Strategy/ConsensusStrategy.php b/Authorization/Strategy/ConsensusStrategy.php index 489b3428..bff56513 100644 --- a/Authorization/Strategy/ConsensusStrategy.php +++ b/Authorization/Strategy/ConsensusStrategy.php @@ -32,13 +32,10 @@ */ final class ConsensusStrategy implements AccessDecisionStrategyInterface, \Stringable { - private bool $allowIfAllAbstainDecisions; - private bool $allowIfEqualGrantedDeniedDecisions; - - public function __construct(bool $allowIfAllAbstainDecisions = false, bool $allowIfEqualGrantedDeniedDecisions = true) - { - $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; - $this->allowIfEqualGrantedDeniedDecisions = $allowIfEqualGrantedDeniedDecisions; + public function __construct( + private bool $allowIfAllAbstainDecisions = false, + private bool $allowIfEqualGrantedDeniedDecisions = true, + ) { } public function decide(\Traversable $results): bool diff --git a/Authorization/Strategy/PriorityStrategy.php b/Authorization/Strategy/PriorityStrategy.php index 9599950c..d7f566ad 100644 --- a/Authorization/Strategy/PriorityStrategy.php +++ b/Authorization/Strategy/PriorityStrategy.php @@ -25,11 +25,9 @@ */ final class PriorityStrategy implements AccessDecisionStrategyInterface, \Stringable { - private bool $allowIfAllAbstainDecisions; - - public function __construct(bool $allowIfAllAbstainDecisions = false) - { - $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; + public function __construct( + private bool $allowIfAllAbstainDecisions = false, + ) { } public function decide(\Traversable $results): bool diff --git a/Authorization/Strategy/UnanimousStrategy.php b/Authorization/Strategy/UnanimousStrategy.php index 1f3b85c5..d47d8994 100644 --- a/Authorization/Strategy/UnanimousStrategy.php +++ b/Authorization/Strategy/UnanimousStrategy.php @@ -24,11 +24,9 @@ */ final class UnanimousStrategy implements AccessDecisionStrategyInterface, \Stringable { - private bool $allowIfAllAbstainDecisions; - - public function __construct(bool $allowIfAllAbstainDecisions = false) - { - $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; + public function __construct( + private bool $allowIfAllAbstainDecisions = false, + ) { } public function decide(\Traversable $results): bool diff --git a/Authorization/TraceableAccessDecisionManager.php b/Authorization/TraceableAccessDecisionManager.php index cb44dce4..17db5461 100644 --- a/Authorization/TraceableAccessDecisionManager.php +++ b/Authorization/TraceableAccessDecisionManager.php @@ -25,17 +25,15 @@ */ class TraceableAccessDecisionManager implements AccessDecisionManagerInterface { - private AccessDecisionManagerInterface $manager; private ?AccessDecisionStrategyInterface $strategy = null; /** @var iterable */ private iterable $voters = []; private array $decisionLog = []; // All decision logs private array $currentLog = []; // Logs being filled in - public function __construct(AccessDecisionManagerInterface $manager) - { - $this->manager = $manager; - + public function __construct( + private AccessDecisionManagerInterface $manager, + ) { // The strategy and voters are stored in a private properties of the decorated service if (property_exists($manager, 'strategy')) { $reflection = new \ReflectionProperty($manager::class, 'strategy'); diff --git a/Authorization/Voter/AuthenticatedVoter.php b/Authorization/Voter/AuthenticatedVoter.php index d7b2b224..a0011868 100644 --- a/Authorization/Voter/AuthenticatedVoter.php +++ b/Authorization/Voter/AuthenticatedVoter.php @@ -33,11 +33,9 @@ class AuthenticatedVoter implements CacheableVoterInterface public const IS_REMEMBERED = 'IS_REMEMBERED'; public const PUBLIC_ACCESS = 'PUBLIC_ACCESS'; - private AuthenticationTrustResolverInterface $authenticationTrustResolver; - - public function __construct(AuthenticationTrustResolverInterface $authenticationTrustResolver) - { - $this->authenticationTrustResolver = $authenticationTrustResolver; + public function __construct( + private AuthenticationTrustResolverInterface $authenticationTrustResolver, + ) { } public function vote(TokenInterface $token, mixed $subject, array $attributes): int diff --git a/Authorization/Voter/ExpressionVoter.php b/Authorization/Voter/ExpressionVoter.php index 6de9c954..bab32830 100644 --- a/Authorization/Voter/ExpressionVoter.php +++ b/Authorization/Voter/ExpressionVoter.php @@ -26,17 +26,12 @@ */ class ExpressionVoter implements CacheableVoterInterface { - private ExpressionLanguage $expressionLanguage; - private AuthenticationTrustResolverInterface $trustResolver; - private AuthorizationCheckerInterface $authChecker; - private ?RoleHierarchyInterface $roleHierarchy; - - public function __construct(ExpressionLanguage $expressionLanguage, AuthenticationTrustResolverInterface $trustResolver, AuthorizationCheckerInterface $authChecker, ?RoleHierarchyInterface $roleHierarchy = null) - { - $this->expressionLanguage = $expressionLanguage; - $this->trustResolver = $trustResolver; - $this->authChecker = $authChecker; - $this->roleHierarchy = $roleHierarchy; + public function __construct( + private ExpressionLanguage $expressionLanguage, + private AuthenticationTrustResolverInterface $trustResolver, + private AuthorizationCheckerInterface $authChecker, + private ?RoleHierarchyInterface $roleHierarchy = null, + ) { } public function supportsAttribute(string $attribute): bool diff --git a/Authorization/Voter/RoleHierarchyVoter.php b/Authorization/Voter/RoleHierarchyVoter.php index 3535ca11..110faa03 100644 --- a/Authorization/Voter/RoleHierarchyVoter.php +++ b/Authorization/Voter/RoleHierarchyVoter.php @@ -22,12 +22,10 @@ */ class RoleHierarchyVoter extends RoleVoter { - private RoleHierarchyInterface $roleHierarchy; - - public function __construct(RoleHierarchyInterface $roleHierarchy, string $prefix = 'ROLE_') - { - $this->roleHierarchy = $roleHierarchy; - + public function __construct( + private RoleHierarchyInterface $roleHierarchy, + string $prefix = 'ROLE_', + ) { parent::__construct($prefix); } diff --git a/Authorization/Voter/RoleVoter.php b/Authorization/Voter/RoleVoter.php index 76de3a32..3c65fb63 100644 --- a/Authorization/Voter/RoleVoter.php +++ b/Authorization/Voter/RoleVoter.php @@ -20,11 +20,9 @@ */ class RoleVoter implements CacheableVoterInterface { - private string $prefix; - - public function __construct(string $prefix = 'ROLE_') - { - $this->prefix = $prefix; + public function __construct( + private string $prefix = 'ROLE_', + ) { } public function vote(TokenInterface $token, mixed $subject, array $attributes): int diff --git a/Authorization/Voter/TraceableVoter.php b/Authorization/Voter/TraceableVoter.php index 412bb976..1abc7c70 100644 --- a/Authorization/Voter/TraceableVoter.php +++ b/Authorization/Voter/TraceableVoter.php @@ -24,13 +24,10 @@ */ class TraceableVoter implements CacheableVoterInterface { - private VoterInterface $voter; - private EventDispatcherInterface $eventDispatcher; - - public function __construct(VoterInterface $voter, EventDispatcherInterface $eventDispatcher) - { - $this->voter = $voter; - $this->eventDispatcher = $eventDispatcher; + public function __construct( + private VoterInterface $voter, + private EventDispatcherInterface $eventDispatcher, + ) { } public function vote(TokenInterface $token, mixed $subject, array $attributes): int diff --git a/Event/AuthenticationEvent.php b/Event/AuthenticationEvent.php index 6fca50d4..f1683558 100644 --- a/Event/AuthenticationEvent.php +++ b/Event/AuthenticationEvent.php @@ -21,15 +21,13 @@ */ class AuthenticationEvent extends Event { - private TokenInterface $authenticationToken; - - public function __construct(TokenInterface $token) - { - $this->authenticationToken = $token; + public function __construct( + private TokenInterface $token, + ) { } public function getAuthenticationToken(): TokenInterface { - return $this->authenticationToken; + return $this->token; } } diff --git a/Event/VoteEvent.php b/Event/VoteEvent.php index 1b1d6a33..edc66b36 100644 --- a/Event/VoteEvent.php +++ b/Event/VoteEvent.php @@ -23,17 +23,12 @@ */ final class VoteEvent extends Event { - private VoterInterface $voter; - private mixed $subject; - private array $attributes; - private int $vote; - - public function __construct(VoterInterface $voter, mixed $subject, array $attributes, int $vote) - { - $this->voter = $voter; - $this->subject = $subject; - $this->attributes = $attributes; - $this->vote = $vote; + public function __construct( + private VoterInterface $voter, + private mixed $subject, + private array $attributes, + private int $vote, + ) { } public function getVoter(): VoterInterface diff --git a/Exception/LazyResponseException.php b/Exception/LazyResponseException.php index e26a3347..a354e68e 100644 --- a/Exception/LazyResponseException.php +++ b/Exception/LazyResponseException.php @@ -20,11 +20,9 @@ */ class LazyResponseException extends \Exception implements ExceptionInterface { - private Response $response; - - public function __construct(Response $response) - { - $this->response = $response; + public function __construct( + private Response $response, + ) { } public function getResponse(): Response diff --git a/Exception/TooManyLoginAttemptsAuthenticationException.php b/Exception/TooManyLoginAttemptsAuthenticationException.php index da1a1a7a..7bb74d64 100644 --- a/Exception/TooManyLoginAttemptsAuthenticationException.php +++ b/Exception/TooManyLoginAttemptsAuthenticationException.php @@ -19,11 +19,9 @@ */ class TooManyLoginAttemptsAuthenticationException extends AuthenticationException { - private ?int $threshold; - - public function __construct(?int $threshold = null) - { - $this->threshold = $threshold; + public function __construct( + private ?int $threshold = null, + ) { } public function getMessageData(): array diff --git a/Role/RoleHierarchy.php b/Role/RoleHierarchy.php index 15c5750d..a2a58457 100644 --- a/Role/RoleHierarchy.php +++ b/Role/RoleHierarchy.php @@ -21,15 +21,12 @@ class RoleHierarchy implements RoleHierarchyInterface /** @var array> */ protected array $map; - private array $hierarchy; - /** * @param array> $hierarchy */ - public function __construct(array $hierarchy) - { - $this->hierarchy = $hierarchy; - + public function __construct( + private array $hierarchy, + ) { $this->buildRoleMap(); } diff --git a/Signature/ExpiredSignatureStorage.php b/Signature/ExpiredSignatureStorage.php index 20803b97..62026644 100644 --- a/Signature/ExpiredSignatureStorage.php +++ b/Signature/ExpiredSignatureStorage.php @@ -18,13 +18,10 @@ */ final class ExpiredSignatureStorage { - private CacheItemPoolInterface $cache; - private int $lifetime; - - public function __construct(CacheItemPoolInterface $cache, int $lifetime) - { - $this->cache = $cache; - $this->lifetime = $lifetime; + public function __construct( + private CacheItemPoolInterface $cache, + private int $lifetime, + ) { } public function countUsages(string $hash): int diff --git a/Signature/SignatureHasher.php b/Signature/SignatureHasher.php index 3f86fce0..b38a449c 100644 --- a/Signature/SignatureHasher.php +++ b/Signature/SignatureHasher.php @@ -25,28 +25,21 @@ */ class SignatureHasher { - private PropertyAccessorInterface $propertyAccessor; - private array $signatureProperties; - private string $secret; - private ?ExpiredSignatureStorage $expiredSignaturesStorage; - private ?int $maxUses; - /** * @param array $signatureProperties Properties of the User; the hash is invalidated if these properties change * @param ExpiredSignatureStorage|null $expiredSignaturesStorage If provided, secures a sequence of hashes that are expired * @param int|null $maxUses Used together with $expiredSignatureStorage to allow a maximum usage of a hash */ - public function __construct(PropertyAccessorInterface $propertyAccessor, array $signatureProperties, #[\SensitiveParameter] string $secret, ?ExpiredSignatureStorage $expiredSignaturesStorage = null, ?int $maxUses = null) - { + public function __construct( + private PropertyAccessorInterface $propertyAccessor, + private array $signatureProperties, + #[\SensitiveParameter] private string $secret, + private ?ExpiredSignatureStorage $expiredSignaturesStorage = null, + private ?int $maxUses = null, + ) { if (!$secret) { throw new InvalidArgumentException('A non-empty secret is required.'); } - - $this->propertyAccessor = $propertyAccessor; - $this->signatureProperties = $signatureProperties; - $this->secret = $secret; - $this->expiredSignaturesStorage = $expiredSignaturesStorage; - $this->maxUses = $maxUses; } /** diff --git a/Test/AccessDecisionStrategyTestCase.php b/Test/AccessDecisionStrategyTestCase.php index bf2a2b9a..85e9fea8 100644 --- a/Test/AccessDecisionStrategyTestCase.php +++ b/Test/AccessDecisionStrategyTestCase.php @@ -64,11 +64,9 @@ final protected static function getVoters(int $grants, int $denies, int $abstain final protected static function getVoter(int $vote): VoterInterface { return new class($vote) implements VoterInterface { - private int $vote; - - public function __construct(int $vote) - { - $this->vote = $vote; + public function __construct( + private int $vote, + ) { } public function vote(TokenInterface $token, $subject, array $attributes): int diff --git a/User/ChainUserProvider.php b/User/ChainUserProvider.php index cef93a2d..f4329e88 100644 --- a/User/ChainUserProvider.php +++ b/User/ChainUserProvider.php @@ -26,14 +26,12 @@ */ class ChainUserProvider implements UserProviderInterface, PasswordUpgraderInterface { - private iterable $providers; - /** * @param iterable $providers */ - public function __construct(iterable $providers) - { - $this->providers = $providers; + public function __construct( + private iterable $providers, + ) { } /** diff --git a/User/InMemoryUser.php b/User/InMemoryUser.php index c319e1f9..5840d0bb 100644 --- a/User/InMemoryUser.php +++ b/User/InMemoryUser.php @@ -22,20 +22,18 @@ final class InMemoryUser implements UserInterface, PasswordAuthenticatedUserInterface, EquatableInterface, \Stringable { private string $username; - private ?string $password; - private bool $enabled; - private array $roles; - public function __construct(?string $username, ?string $password, array $roles = [], bool $enabled = true) - { + public function __construct( + ?string $username, + private ?string $password, + private array $roles = [], + private bool $enabled = true, + ) { if ('' === $username || null === $username) { throw new \InvalidArgumentException('The username cannot be empty.'); } $this->username = $username; - $this->password = $password; - $this->enabled = $enabled; - $this->roles = $roles; } public function __toString(): string diff --git a/Validator/Constraints/UserPasswordValidator.php b/Validator/Constraints/UserPasswordValidator.php index 3d6c7637..c3869824 100644 --- a/Validator/Constraints/UserPasswordValidator.php +++ b/Validator/Constraints/UserPasswordValidator.php @@ -22,13 +22,10 @@ class UserPasswordValidator extends ConstraintValidator { - private TokenStorageInterface $tokenStorage; - private PasswordHasherFactoryInterface $hasherFactory; - - public function __construct(TokenStorageInterface $tokenStorage, PasswordHasherFactoryInterface $hasherFactory) - { - $this->tokenStorage = $tokenStorage; - $this->hasherFactory = $hasherFactory; + public function __construct( + private TokenStorageInterface $tokenStorage, + private PasswordHasherFactoryInterface $hasherFactory, + ) { } public function validate(mixed $password, Constraint $constraint): void From e70f73a3c359bb09d77e624579f5d0282bb25f19 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Thu, 20 Jun 2024 17:52:34 +0200 Subject: [PATCH 24/52] Prefix all sprintf() calls --- Authentication/Token/AbstractToken.php | 4 ++-- Authorization/AccessDecisionManager.php | 4 ++-- Authorization/ExpressionLanguage.php | 2 +- Authorization/ExpressionLanguageProvider.php | 2 +- Signature/SignatureHasher.php | 4 ++-- Tests/Resources/TranslationFilesTest.php | 4 ++-- User/ChainUserProvider.php | 6 +++--- User/InMemoryUserProvider.php | 6 +++--- User/MissingUserProvider.php | 2 +- Validator/Constraints/UserPasswordValidator.php | 2 +- 10 files changed, 18 insertions(+), 18 deletions(-) diff --git a/Authentication/Token/AbstractToken.php b/Authentication/Token/AbstractToken.php index 36d64766..67d992ce 100644 --- a/Authentication/Token/AbstractToken.php +++ b/Authentication/Token/AbstractToken.php @@ -125,7 +125,7 @@ public function hasAttribute(string $name): bool public function getAttribute(string $name): mixed { if (!\array_key_exists($name, $this->attributes)) { - throw new \InvalidArgumentException(sprintf('This token has no "%s" attribute.', $name)); + throw new \InvalidArgumentException(\sprintf('This token has no "%s" attribute.', $name)); } return $this->attributes[$name]; @@ -146,7 +146,7 @@ public function __toString(): string $roles[] = $role; } - return sprintf('%s(user="%s", roles="%s")', $class, $this->getUserIdentifier(), implode(', ', $roles)); + return \sprintf('%s(user="%s", roles="%s")', $class, $this->getUserIdentifier(), implode(', ', $roles)); } /** diff --git a/Authorization/AccessDecisionManager.php b/Authorization/AccessDecisionManager.php index 10969acb..3e42c4bf 100644 --- a/Authorization/AccessDecisionManager.php +++ b/Authorization/AccessDecisionManager.php @@ -53,7 +53,7 @@ public function decide(TokenInterface $token, array $attributes, mixed $object = { // Special case for AccessListener, do not remove the right side of the condition before 6.0 if (\count($attributes) > 1 && !$allowMultipleAttributes) { - throw new InvalidArgumentException(sprintf('Passing more than one Security attribute to "%s()" is not supported.', __METHOD__)); + throw new InvalidArgumentException(\sprintf('Passing more than one Security attribute to "%s()" is not supported.', __METHOD__)); } return $this->strategy->decide( @@ -69,7 +69,7 @@ private function collectResults(TokenInterface $token, array $attributes, mixed foreach ($this->getVoters($attributes, $object) as $voter) { $result = $voter->vote($token, $object, $attributes); if (!\is_int($result) || !(self::VALID_VOTES[$result] ?? false)) { - throw new \LogicException(sprintf('"%s::vote()" must return one of "%s" constants ("ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN"), "%s" returned.', get_debug_type($voter), VoterInterface::class, var_export($result, true))); + throw new \LogicException(\sprintf('"%s::vote()" must return one of "%s" constants ("ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN"), "%s" returned.', get_debug_type($voter), VoterInterface::class, var_export($result, true))); } yield $result; diff --git a/Authorization/ExpressionLanguage.php b/Authorization/ExpressionLanguage.php index a48d8148..846d2cf6 100644 --- a/Authorization/ExpressionLanguage.php +++ b/Authorization/ExpressionLanguage.php @@ -15,7 +15,7 @@ use Symfony\Component\ExpressionLanguage\ExpressionLanguage as BaseExpressionLanguage; if (!class_exists(BaseExpressionLanguage::class)) { - throw new \LogicException(sprintf('The "%s" class requires the "ExpressionLanguage" component. Try running "composer require symfony/expression-language".', ExpressionLanguage::class)); + throw new \LogicException(\sprintf('The "%s" class requires the "ExpressionLanguage" component. Try running "composer require symfony/expression-language".', ExpressionLanguage::class)); } else { // Help opcache.preload discover always-needed symbols class_exists(ExpressionLanguageProvider::class); diff --git a/Authorization/ExpressionLanguageProvider.php b/Authorization/ExpressionLanguageProvider.php index d3e2dac0..2e558c21 100644 --- a/Authorization/ExpressionLanguageProvider.php +++ b/Authorization/ExpressionLanguageProvider.php @@ -28,7 +28,7 @@ public function getFunctions(): array new ExpressionFunction('is_fully_authenticated', fn () => '$token && $auth_checker->isGranted("IS_AUTHENTICATED_FULLY")', fn (array $variables) => $variables['token'] && $variables['auth_checker']->isGranted('IS_AUTHENTICATED_FULLY')), - new ExpressionFunction('is_granted', fn ($attributes, $object = 'null') => sprintf('$auth_checker->isGranted(%s, %s)', $attributes, $object), fn (array $variables, $attributes, $object = null) => $variables['auth_checker']->isGranted($attributes, $object)), + new ExpressionFunction('is_granted', fn ($attributes, $object = 'null') => \sprintf('$auth_checker->isGranted(%s, %s)', $attributes, $object), fn (array $variables, $attributes, $object = null) => $variables['auth_checker']->isGranted($attributes, $object)), new ExpressionFunction('is_remember_me', fn () => '$token && $auth_checker->isGranted("IS_REMEMBERED")', fn (array $variables) => $variables['token'] && $variables['auth_checker']->isGranted('IS_REMEMBERED')), ]; diff --git a/Signature/SignatureHasher.php b/Signature/SignatureHasher.php index b38a449c..903f5b34 100644 --- a/Signature/SignatureHasher.php +++ b/Signature/SignatureHasher.php @@ -87,7 +87,7 @@ public function verifySignatureHash(UserInterface $user, int $expires, string $h if ($this->expiredSignaturesStorage && $this->maxUses) { if ($this->expiredSignaturesStorage->countUsages($hash) >= $this->maxUses) { - throw new ExpiredSignatureException(sprintf('Signature can only be used "%d" times.', $this->maxUses)); + throw new ExpiredSignatureException(\sprintf('Signature can only be used "%d" times.', $this->maxUses)); } $this->expiredSignaturesStorage->incrementUsages($hash); @@ -111,7 +111,7 @@ public function computeSignatureHash(UserInterface $user, int $expires): string } if (!\is_scalar($value) && !$value instanceof \Stringable) { - throw new \InvalidArgumentException(sprintf('The property path "%s" on the user object "%s" must return a value that can be cast to a string, but "%s" was returned.', $property, $user::class, get_debug_type($value))); + throw new \InvalidArgumentException(\sprintf('The property path "%s" on the user object "%s" must return a value that can be cast to a string, but "%s" was returned.', $property, $user::class, get_debug_type($value))); } hash_update($fieldsHash, ':'.base64_encode($value)); } diff --git a/Tests/Resources/TranslationFilesTest.php b/Tests/Resources/TranslationFilesTest.php index 6bd9a21d..695cdd98 100644 --- a/Tests/Resources/TranslationFilesTest.php +++ b/Tests/Resources/TranslationFilesTest.php @@ -26,7 +26,7 @@ public function testTranslationFileIsValid($filePath) $errors = XliffUtils::validateSchema($document); - $this->assertCount(0, $errors, sprintf('"%s" is invalid:%s', $filePath, \PHP_EOL.implode(\PHP_EOL, array_column($errors, 'message')))); + $this->assertCount(0, $errors, \sprintf('"%s" is invalid:%s', $filePath, \PHP_EOL.implode(\PHP_EOL, array_column($errors, 'message')))); } /** @@ -39,7 +39,7 @@ public function testTranslationFileIsValidWithoutEntityLoader($filePath) $errors = XliffUtils::validateSchema($document); - $this->assertCount(0, $errors, sprintf('"%s" is invalid:%s', $filePath, \PHP_EOL.implode(\PHP_EOL, array_column($errors, 'message')))); + $this->assertCount(0, $errors, \sprintf('"%s" is invalid:%s', $filePath, \PHP_EOL.implode(\PHP_EOL, array_column($errors, 'message')))); } public static function provideTranslationFiles() diff --git a/User/ChainUserProvider.php b/User/ChainUserProvider.php index f4329e88..484d0d47 100644 --- a/User/ChainUserProvider.php +++ b/User/ChainUserProvider.php @@ -56,7 +56,7 @@ public function loadUserByIdentifier(string $identifier): UserInterface } } - $ex = new UserNotFoundException(sprintf('There is no user with identifier "%s".', $identifier)); + $ex = new UserNotFoundException(\sprintf('There is no user with identifier "%s".', $identifier)); $ex->setUserIdentifier($identifier); throw $ex; } @@ -82,11 +82,11 @@ public function refreshUser(UserInterface $user): UserInterface if ($supportedUserFound) { $username = $user->getUserIdentifier(); - $e = new UserNotFoundException(sprintf('There is no user with name "%s".', $username)); + $e = new UserNotFoundException(\sprintf('There is no user with name "%s".', $username)); $e->setUserIdentifier($username); throw $e; } else { - throw new UnsupportedUserException(sprintf('There is no user provider for user "%s". Shouldn\'t the "supportsClass()" method of your user provider return true for this classname?', get_debug_type($user))); + throw new UnsupportedUserException(\sprintf('There is no user provider for user "%s". Shouldn\'t the "supportsClass()" method of your user provider return true for this classname?', get_debug_type($user))); } } diff --git a/User/InMemoryUserProvider.php b/User/InMemoryUserProvider.php index 04bf682a..db6267a5 100644 --- a/User/InMemoryUserProvider.php +++ b/User/InMemoryUserProvider.php @@ -55,7 +55,7 @@ public function __construct(array $users = []) public function createUser(UserInterface $user): void { if (!$user instanceof InMemoryUser) { - throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_debug_type($user))); + throw new UnsupportedUserException(\sprintf('Instances of "%s" are not supported.', get_debug_type($user))); } $userIdentifier = strtolower($user->getUserIdentifier()); @@ -76,7 +76,7 @@ public function loadUserByIdentifier(string $identifier): UserInterface public function refreshUser(UserInterface $user): UserInterface { if (!$user instanceof InMemoryUser) { - throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_debug_type($user))); + throw new UnsupportedUserException(\sprintf('Instances of "%s" are not supported.', get_debug_type($user))); } $storedUser = $this->getUser($user->getUserIdentifier()); @@ -98,7 +98,7 @@ public function supportsClass(string $class): bool private function getUser(string $username): InMemoryUser { if (!isset($this->users[strtolower($username)])) { - $ex = new UserNotFoundException(sprintf('Username "%s" does not exist.', $username)); + $ex = new UserNotFoundException(\sprintf('Username "%s" does not exist.', $username)); $ex->setUserIdentifier($username); throw $ex; diff --git a/User/MissingUserProvider.php b/User/MissingUserProvider.php index cf6102aa..9869259a 100644 --- a/User/MissingUserProvider.php +++ b/User/MissingUserProvider.php @@ -28,7 +28,7 @@ class MissingUserProvider implements UserProviderInterface */ public function __construct(string $firewall) { - throw new InvalidConfigurationException(sprintf('"%s" firewall requires a user provider but none was defined.', $firewall)); + throw new InvalidConfigurationException(\sprintf('"%s" firewall requires a user provider but none was defined.', $firewall)); } public function loadUserByUsername(string $username): UserInterface diff --git a/Validator/Constraints/UserPasswordValidator.php b/Validator/Constraints/UserPasswordValidator.php index c3869824..6c9bdef9 100644 --- a/Validator/Constraints/UserPasswordValidator.php +++ b/Validator/Constraints/UserPasswordValidator.php @@ -49,7 +49,7 @@ public function validate(mixed $password, Constraint $constraint): void $user = $this->tokenStorage->getToken()->getUser(); if (!$user instanceof PasswordAuthenticatedUserInterface) { - throw new ConstraintDefinitionException(sprintf('The "%s" class must implement the "%s" interface.', PasswordAuthenticatedUserInterface::class, get_debug_type($user))); + throw new ConstraintDefinitionException(\sprintf('The "%s" class must implement the "%s" interface.', PasswordAuthenticatedUserInterface::class, get_debug_type($user))); } $hasher = $this->hasherFactory->getPasswordHasher($user); From 40cbdcaad146cd431686dddaf5231f13579f1d00 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 6 Jul 2024 09:57:16 +0200 Subject: [PATCH 25/52] Update .gitattributes --- .gitattributes | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.gitattributes b/.gitattributes index 84c7add0..14c3c359 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,4 +1,3 @@ /Tests export-ignore /phpunit.xml.dist export-ignore -/.gitattributes export-ignore -/.gitignore export-ignore +/.git* export-ignore From 138cfe23ea2e991f3c9999c077779dcdce03e0f6 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Fri, 19 Jul 2024 09:42:17 +0200 Subject: [PATCH 26/52] pass the current token to the checkPostAuth() method of user checkers --- CHANGELOG.md | 1 + User/ChainUserChecker.php | 12 ++++++++++-- User/UserCheckerInterface.php | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 208f0d48..ac99a3c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG 7.2 --- + * Add `$token` argument to `UserCheckerInterface::checkPostAuth()` * Deprecate argument `$secret` of `RememberMeToken` 7.0 diff --git a/User/ChainUserChecker.php b/User/ChainUserChecker.php index f889d35d..67fd76b9 100644 --- a/User/ChainUserChecker.php +++ b/User/ChainUserChecker.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Security\Core\User; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; + final class ChainUserChecker implements UserCheckerInterface { /** @@ -27,10 +29,16 @@ public function checkPreAuth(UserInterface $user): void } } - public function checkPostAuth(UserInterface $user): void + public function checkPostAuth(UserInterface $user /*, TokenInterface $token*/): void { + $token = 1 < \func_num_args() ? func_get_arg(1) : null; + foreach ($this->checkers as $checker) { - $checker->checkPostAuth($user); + if ($token instanceof TokenInterface) { + $checker->checkPostAuth($user, $token); + } else { + $checker->checkPostAuth($user); + } } } } diff --git a/User/UserCheckerInterface.php b/User/UserCheckerInterface.php index 480ba7b5..2dc748aa 100644 --- a/User/UserCheckerInterface.php +++ b/User/UserCheckerInterface.php @@ -35,5 +35,5 @@ public function checkPreAuth(UserInterface $user): void; * * @throws AccountStatusException */ - public function checkPostAuth(UserInterface $user): void; + public function checkPostAuth(UserInterface $user /*, TokenInterface $token*/): void; } From 9b1348ac4bc9f25b6d9ccfd7e5e6b3af1cf78c0b Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Wed, 31 Jul 2024 16:13:26 +0200 Subject: [PATCH 27/52] Remove unused code and unnecessary `else` branches --- User/ChainUserProvider.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/User/ChainUserProvider.php b/User/ChainUserProvider.php index 484d0d47..21e2e618 100644 --- a/User/ChainUserProvider.php +++ b/User/ChainUserProvider.php @@ -85,9 +85,9 @@ public function refreshUser(UserInterface $user): UserInterface $e = new UserNotFoundException(\sprintf('There is no user with name "%s".', $username)); $e->setUserIdentifier($username); throw $e; - } else { - throw new UnsupportedUserException(\sprintf('There is no user provider for user "%s". Shouldn\'t the "supportsClass()" method of your user provider return true for this classname?', get_debug_type($user))); } + + throw new UnsupportedUserException(\sprintf('There is no user provider for user "%s". Shouldn\'t the "supportsClass()" method of your user provider return true for this classname?', get_debug_type($user))); } public function supportsClass(string $class): bool From f0267ab90e6edb18ec982ad481c867c7fb3375a9 Mon Sep 17 00:00:00 2001 From: Roy de Vos Burchart Date: Thu, 1 Aug 2024 17:21:17 +0200 Subject: [PATCH 28/52] Code style change in `@PER-CS2.0` affecting `@Symfony` (parentheses for anonymous classes) --- Tests/Authorization/AccessDecisionManagerTest.php | 2 +- Tests/Authorization/Voter/AuthenticatedVoterTest.php | 2 +- Tests/Authorization/Voter/VoterTest.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/Authorization/AccessDecisionManagerTest.php b/Tests/Authorization/AccessDecisionManagerTest.php index b06a7a79..f0a49788 100644 --- a/Tests/Authorization/AccessDecisionManagerTest.php +++ b/Tests/Authorization/AccessDecisionManagerTest.php @@ -39,7 +39,7 @@ public function testVoterCalls() $this->getUnexpectedVoter(), ]; - $strategy = new class() implements AccessDecisionStrategyInterface { + $strategy = new class implements AccessDecisionStrategyInterface { public function decide(\Traversable $results): bool { $i = 0; diff --git a/Tests/Authorization/Voter/AuthenticatedVoterTest.php b/Tests/Authorization/Voter/AuthenticatedVoterTest.php index 3a3b9d4e..ed894b3a 100644 --- a/Tests/Authorization/Voter/AuthenticatedVoterTest.php +++ b/Tests/Authorization/Voter/AuthenticatedVoterTest.php @@ -90,7 +90,7 @@ protected function getToken($authenticated) $user = new InMemoryUser('wouter', '', ['ROLE_USER']); if ('fully' === $authenticated) { - $token = new class() extends AbstractToken { + $token = new class extends AbstractToken { public function getCredentials() { } diff --git a/Tests/Authorization/Voter/VoterTest.php b/Tests/Authorization/Voter/VoterTest.php index 5636340e..602c61ab 100644 --- a/Tests/Authorization/Voter/VoterTest.php +++ b/Tests/Authorization/Voter/VoterTest.php @@ -41,7 +41,7 @@ public static function getTests(): array [$voter, ['DELETE'], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attribute is supported'], - [$voter, ['EDIT'], VoterInterface::ACCESS_ABSTAIN, new class() {}, 'ACCESS_ABSTAIN if class is not supported'], + [$voter, ['EDIT'], VoterInterface::ACCESS_ABSTAIN, new class {}, 'ACCESS_ABSTAIN if class is not supported'], [$voter, ['EDIT'], VoterInterface::ACCESS_ABSTAIN, null, 'ACCESS_ABSTAIN if object is null'], From a32dd8b58706d9d5be2ceeb2873276d600235a88 Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Mon, 29 Jul 2024 09:33:48 +0200 Subject: [PATCH 29/52] Remove useless code --- User/InMemoryUser.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/User/InMemoryUser.php b/User/InMemoryUser.php index 5840d0bb..b14bc077 100644 --- a/User/InMemoryUser.php +++ b/User/InMemoryUser.php @@ -88,8 +88,8 @@ public function isEqualTo(UserInterface $user): bool return false; } - $currentRoles = array_map('strval', (array) $this->getRoles()); - $newRoles = array_map('strval', (array) $user->getRoles()); + $currentRoles = array_map('strval', $this->getRoles()); + $newRoles = array_map('strval', $user->getRoles()); $rolesChanged = \count($currentRoles) !== \count($newRoles) || \count($currentRoles) !== \count(array_intersect($currentRoles, $newRoles)); if ($rolesChanged) { return false; From 0d94fd9669fe990d23d7d27ecb1e8448a025870c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20J=2E=20Garc=C3=ADa=20Lagar?= Date: Wed, 14 Aug 2024 08:46:31 +0200 Subject: [PATCH 30/52] Deprecate empty user identifier --- CHANGELOG.md | 1 + User/UserInterface.php | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ac99a3c0..5dd5ef38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ CHANGELOG * Add `$token` argument to `UserCheckerInterface::checkPostAuth()` * Deprecate argument `$secret` of `RememberMeToken` + * Deprecate returning an empty string in `UserInterface::getUserIdentifier()` 7.0 --- diff --git a/User/UserInterface.php b/User/UserInterface.php index 50f8fb0f..e6078399 100644 --- a/User/UserInterface.php +++ b/User/UserInterface.php @@ -56,6 +56,8 @@ public function eraseCredentials(): void; /** * Returns the identifier for this user (e.g. username or email address). + * + * @return non-empty-string */ public function getUserIdentifier(): string; } From 2acef5cfd7cc6697d4618cdd9eb363eb5ab713b2 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 26 Aug 2024 17:35:30 +0200 Subject: [PATCH 31/52] Use Stringable whenever possible --- Authorization/TraceableAccessDecisionManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Authorization/TraceableAccessDecisionManager.php b/Authorization/TraceableAccessDecisionManager.php index 17db5461..0b82eb3a 100644 --- a/Authorization/TraceableAccessDecisionManager.php +++ b/Authorization/TraceableAccessDecisionManager.php @@ -85,7 +85,7 @@ public function getStrategy(): string if (null === $this->strategy) { return '-'; } - if (method_exists($this->strategy, '__toString')) { + if ($this->strategy instanceof \Stringable) { return (string) $this->strategy; } From c563eafb17f8332faa9d600a82b7d732f22d3ae6 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Thu, 5 Sep 2024 11:40:18 +0200 Subject: [PATCH 32/52] make test case classes compatible with PHPUnit 10+ --- CHANGELOG.md | 1 + Test/AccessDecisionStrategyTestCase.php | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dd5ef38..7cf09c70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG 7.2 --- + * Make `AccessDecisionStrategyTestCase` compatible with PHPUnit 10+ * Add `$token` argument to `UserCheckerInterface::checkPostAuth()` * Deprecate argument `$secret` of `RememberMeToken` * Deprecate returning an empty string in `UserInterface::getUserIdentifier()` diff --git a/Test/AccessDecisionStrategyTestCase.php b/Test/AccessDecisionStrategyTestCase.php index 85e9fea8..792e7779 100644 --- a/Test/AccessDecisionStrategyTestCase.php +++ b/Test/AccessDecisionStrategyTestCase.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Security\Core\Test; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionManager; @@ -29,6 +30,7 @@ abstract class AccessDecisionStrategyTestCase extends TestCase * * @param VoterInterface[] $voters */ + #[DataProvider('provideStrategyTests')] final public function testDecide(AccessDecisionStrategyInterface $strategy, array $voters, bool $expected) { $token = $this->createMock(TokenInterface::class); From 312a726ac8b92bf1544355eefd2d290211d3119b Mon Sep 17 00:00:00 2001 From: Nate Wiebe Date: Mon, 13 Dec 2021 17:05:08 -0500 Subject: [PATCH 33/52] [Security][SecurityBundle] User authorization checker --- .../Token/OfflineTokenInterface.php | 21 ++++++ .../Token/UserAuthorizationCheckerToken.php | 31 ++++++++ Authorization/UserAuthorizationChecker.php | 31 ++++++++ .../UserAuthorizationCheckerInterface.php | 29 ++++++++ Authorization/Voter/AuthenticatedVoter.php | 6 ++ CHANGELOG.md | 7 ++ .../UserAuthorizationCheckerTokenTest.php | 26 +++++++ .../UserAuthorizationCheckerTest.php | 70 +++++++++++++++++++ .../Voter/AuthenticatedVoterTest.php | 43 ++++++++++++ 9 files changed, 264 insertions(+) create mode 100644 Authentication/Token/OfflineTokenInterface.php create mode 100644 Authentication/Token/UserAuthorizationCheckerToken.php create mode 100644 Authorization/UserAuthorizationChecker.php create mode 100644 Authorization/UserAuthorizationCheckerInterface.php create mode 100644 Tests/Authentication/Token/UserAuthorizationCheckerTokenTest.php create mode 100644 Tests/Authorization/UserAuthorizationCheckerTest.php diff --git a/Authentication/Token/OfflineTokenInterface.php b/Authentication/Token/OfflineTokenInterface.php new file mode 100644 index 00000000..894f0fd1 --- /dev/null +++ b/Authentication/Token/OfflineTokenInterface.php @@ -0,0 +1,21 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Authentication\Token; + +/** + * Interface used for marking tokens that do not represent the currently logged-in user. + * + * @author Nate Wiebe + */ +interface OfflineTokenInterface extends TokenInterface +{ +} diff --git a/Authentication/Token/UserAuthorizationCheckerToken.php b/Authentication/Token/UserAuthorizationCheckerToken.php new file mode 100644 index 00000000..2e84ce7a --- /dev/null +++ b/Authentication/Token/UserAuthorizationCheckerToken.php @@ -0,0 +1,31 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Authentication\Token; + +use Symfony\Component\Security\Core\User\UserInterface; + +/** + * UserAuthorizationCheckerToken implements a token used for checking authorization. + * + * @author Nate Wiebe + * + * @internal + */ +final class UserAuthorizationCheckerToken extends AbstractToken implements OfflineTokenInterface +{ + public function __construct(UserInterface $user) + { + parent::__construct($user->getRoles()); + + $this->setUser($user); + } +} diff --git a/Authorization/UserAuthorizationChecker.php b/Authorization/UserAuthorizationChecker.php new file mode 100644 index 00000000..e4d2eab6 --- /dev/null +++ b/Authorization/UserAuthorizationChecker.php @@ -0,0 +1,31 @@ + + * + * 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; + +use Symfony\Component\Security\Core\Authentication\Token\UserAuthorizationCheckerToken; +use Symfony\Component\Security\Core\User\UserInterface; + +/** + * @author Nate Wiebe + */ +final class UserAuthorizationChecker implements UserAuthorizationCheckerInterface +{ + public function __construct( + private readonly AccessDecisionManagerInterface $accessDecisionManager, + ) { + } + + public function userIsGranted(UserInterface $user, mixed $attribute, mixed $subject = null): bool + { + return $this->accessDecisionManager->decide(new UserAuthorizationCheckerToken($user), [$attribute], $subject); + } +} diff --git a/Authorization/UserAuthorizationCheckerInterface.php b/Authorization/UserAuthorizationCheckerInterface.php new file mode 100644 index 00000000..370cf61a --- /dev/null +++ b/Authorization/UserAuthorizationCheckerInterface.php @@ -0,0 +1,29 @@ + + * + * 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; + +use Symfony\Component\Security\Core\User\UserInterface; + +/** + * Interface is used to check user authorization without a session. + * + * @author Nate Wiebe + */ +interface UserAuthorizationCheckerInterface +{ + /** + * Checks if the attribute is granted against the user and optionally supplied subject. + * + * @param mixed $attribute A single attribute to vote on (can be of any type, string and instance of Expression are supported by the core) + */ + public function userIsGranted(UserInterface $user, mixed $attribute, mixed $subject = null): bool; +} diff --git a/Authorization/Voter/AuthenticatedVoter.php b/Authorization/Voter/AuthenticatedVoter.php index a0011868..a073f616 100644 --- a/Authorization/Voter/AuthenticatedVoter.php +++ b/Authorization/Voter/AuthenticatedVoter.php @@ -12,8 +12,10 @@ namespace Symfony\Component\Security\Core\Authorization\Voter; use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface; +use Symfony\Component\Security\Core\Authentication\Token\OfflineTokenInterface; use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Exception\InvalidArgumentException; /** * AuthenticatedVoter votes if an attribute like IS_AUTHENTICATED_FULLY, @@ -54,6 +56,10 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes): continue; } + if ($token instanceof OfflineTokenInterface) { + throw new InvalidArgumentException('Cannot decide on authentication attributes when an offline token is used.'); + } + $result = VoterInterface::ACCESS_DENIED; if (self::IS_AUTHENTICATED_FULLY === $attribute diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cf09c70..2a54af9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ CHANGELOG ========= +7.3 +--- + + * Add `UserAuthorizationChecker::userIsGranted()` to test user authorization without relying on the session. + For example, users not currently logged in, or while processing a message from a message queue. + * Add `OfflineTokenInterface` to mark tokens that do not represent the currently logged-in user + 7.2 --- diff --git a/Tests/Authentication/Token/UserAuthorizationCheckerTokenTest.php b/Tests/Authentication/Token/UserAuthorizationCheckerTokenTest.php new file mode 100644 index 00000000..2e7e11bd --- /dev/null +++ b/Tests/Authentication/Token/UserAuthorizationCheckerTokenTest.php @@ -0,0 +1,26 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Tests\Authentication\Token; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\Security\Core\Authentication\Token\UserAuthorizationCheckerToken; +use Symfony\Component\Security\Core\User\InMemoryUser; + +class UserAuthorizationCheckerTokenTest extends TestCase +{ + public function testConstructor() + { + $token = new UserAuthorizationCheckerToken($user = new InMemoryUser('foo', 'bar', ['ROLE_FOO'])); + $this->assertSame(['ROLE_FOO'], $token->getRoleNames()); + $this->assertSame($user, $token->getUser()); + } +} diff --git a/Tests/Authorization/UserAuthorizationCheckerTest.php b/Tests/Authorization/UserAuthorizationCheckerTest.php new file mode 100644 index 00000000..e8b165a6 --- /dev/null +++ b/Tests/Authorization/UserAuthorizationCheckerTest.php @@ -0,0 +1,70 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Tests\Authorization; + +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; +use Symfony\Component\Security\Core\Authentication\Token\UserAuthorizationCheckerToken; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; +use Symfony\Component\Security\Core\Authorization\UserAuthorizationChecker; +use Symfony\Component\Security\Core\User\InMemoryUser; + +class UserAuthorizationCheckerTest extends TestCase +{ + private AccessDecisionManagerInterface&MockObject $accessDecisionManager; + private UserAuthorizationChecker $authorizationChecker; + + protected function setUp(): void + { + $this->accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + + $this->authorizationChecker = new UserAuthorizationChecker($this->accessDecisionManager); + } + + /** + * @dataProvider isGrantedProvider + */ + public function testIsGranted(bool $decide, array $roles) + { + $user = new InMemoryUser('username', 'password', $roles); + + $this->accessDecisionManager + ->expects($this->once()) + ->method('decide') + ->with($this->callback(fn (UserAuthorizationCheckerToken $token): bool => $user === $token->getUser()), $this->identicalTo(['ROLE_FOO'])) + ->willReturn($decide); + + $this->assertSame($decide, $this->authorizationChecker->userIsGranted($user, 'ROLE_FOO')); + } + + public static function isGrantedProvider(): array + { + return [ + [false, ['ROLE_USER']], + [true, ['ROLE_USER', 'ROLE_FOO']], + ]; + } + + public function testIsGrantedWithObjectAttribute() + { + $attribute = new \stdClass(); + + $token = new UserAuthorizationCheckerToken(new InMemoryUser('username', 'password', ['ROLE_USER'])); + + $this->accessDecisionManager + ->expects($this->once()) + ->method('decide') + ->with($this->isInstanceOf($token::class), $this->identicalTo([$attribute])) + ->willReturn(true); + $this->assertTrue($this->authorizationChecker->userIsGranted($token->getUser(), $attribute)); + } +} diff --git a/Tests/Authorization/Voter/AuthenticatedVoterTest.php b/Tests/Authorization/Voter/AuthenticatedVoterTest.php index ed894b3a..89f6c350 100644 --- a/Tests/Authorization/Voter/AuthenticatedVoterTest.php +++ b/Tests/Authorization/Voter/AuthenticatedVoterTest.php @@ -17,8 +17,10 @@ use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken; use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; +use Symfony\Component\Security\Core\Authentication\Token\UserAuthorizationCheckerToken; use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; +use Symfony\Component\Security\Core\Exception\InvalidArgumentException; use Symfony\Component\Security\Core\User\InMemoryUser; class AuthenticatedVoterTest extends TestCase @@ -85,6 +87,43 @@ public function testSupportsType() $this->assertTrue($voter->supportsType(get_debug_type(new \stdClass()))); } + /** + * @dataProvider provideOfflineAttributes + */ + public function testOfflineToken($attributes, $expected) + { + $voter = new AuthenticatedVoter(new AuthenticationTrustResolver()); + + $this->assertSame($expected, $voter->vote($this->getToken('offline'), null, $attributes)); + } + + public static function provideOfflineAttributes() + { + yield [[AuthenticatedVoter::PUBLIC_ACCESS], VoterInterface::ACCESS_GRANTED]; + yield [['ROLE_FOO'], VoterInterface::ACCESS_ABSTAIN]; + } + + /** + * @dataProvider provideUnsupportedOfflineAttributes + */ + public function testUnsupportedOfflineToken(string $attribute) + { + $voter = new AuthenticatedVoter(new AuthenticationTrustResolver()); + + $this->expectException(InvalidArgumentException::class); + + $voter->vote($this->getToken('offline'), null, [$attribute]); + } + + public static function provideUnsupportedOfflineAttributes() + { + yield [AuthenticatedVoter::IS_AUTHENTICATED_FULLY]; + yield [AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED]; + yield [AuthenticatedVoter::IS_AUTHENTICATED]; + yield [AuthenticatedVoter::IS_IMPERSONATOR]; + yield [AuthenticatedVoter::IS_REMEMBERED]; + } + protected function getToken($authenticated) { $user = new InMemoryUser('wouter', '', ['ROLE_USER']); @@ -108,6 +147,10 @@ public function getCredentials() return $this->getMockBuilder(SwitchUserToken::class)->disableOriginalConstructor()->getMock(); } + if ('offline' === $authenticated) { + return new UserAuthorizationCheckerToken($user); + } + return new NullToken(); } } From 9a04c4c5bf59a255f177b588bc2b3e1ab1683e57 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Sun, 15 Dec 2024 11:42:01 +0100 Subject: [PATCH 34/52] rename userIsGranted() to isGrantedForUser() --- Authorization/UserAuthorizationChecker.php | 2 +- Authorization/UserAuthorizationCheckerInterface.php | 2 +- CHANGELOG.md | 2 +- Tests/Authorization/UserAuthorizationCheckerTest.php | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Authorization/UserAuthorizationChecker.php b/Authorization/UserAuthorizationChecker.php index e4d2eab6..f5ba7b88 100644 --- a/Authorization/UserAuthorizationChecker.php +++ b/Authorization/UserAuthorizationChecker.php @@ -24,7 +24,7 @@ public function __construct( ) { } - public function userIsGranted(UserInterface $user, mixed $attribute, mixed $subject = null): bool + public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null): bool { return $this->accessDecisionManager->decide(new UserAuthorizationCheckerToken($user), [$attribute], $subject); } diff --git a/Authorization/UserAuthorizationCheckerInterface.php b/Authorization/UserAuthorizationCheckerInterface.php index 370cf61a..3335e6fd 100644 --- a/Authorization/UserAuthorizationCheckerInterface.php +++ b/Authorization/UserAuthorizationCheckerInterface.php @@ -25,5 +25,5 @@ interface UserAuthorizationCheckerInterface * * @param mixed $attribute A single attribute to vote on (can be of any type, string and instance of Expression are supported by the core) */ - public function userIsGranted(UserInterface $user, mixed $attribute, mixed $subject = null): bool; + public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null): bool; } diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a54af9e..3cc738ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ CHANGELOG 7.3 --- - * Add `UserAuthorizationChecker::userIsGranted()` to test user authorization without relying on the session. + * Add `UserAuthorizationChecker::isGrantedForUser()` to test user authorization without relying on the session. For example, users not currently logged in, or while processing a message from a message queue. * Add `OfflineTokenInterface` to mark tokens that do not represent the currently logged-in user diff --git a/Tests/Authorization/UserAuthorizationCheckerTest.php b/Tests/Authorization/UserAuthorizationCheckerTest.php index e8b165a6..e9b6bb74 100644 --- a/Tests/Authorization/UserAuthorizationCheckerTest.php +++ b/Tests/Authorization/UserAuthorizationCheckerTest.php @@ -43,7 +43,7 @@ public function testIsGranted(bool $decide, array $roles) ->with($this->callback(fn (UserAuthorizationCheckerToken $token): bool => $user === $token->getUser()), $this->identicalTo(['ROLE_FOO'])) ->willReturn($decide); - $this->assertSame($decide, $this->authorizationChecker->userIsGranted($user, 'ROLE_FOO')); + $this->assertSame($decide, $this->authorizationChecker->isGrantedForUser($user, 'ROLE_FOO')); } public static function isGrantedProvider(): array @@ -65,6 +65,6 @@ public function testIsGrantedWithObjectAttribute() ->method('decide') ->with($this->isInstanceOf($token::class), $this->identicalTo([$attribute])) ->willReturn(true); - $this->assertTrue($this->authorizationChecker->userIsGranted($token->getUser(), $attribute)); + $this->assertTrue($this->authorizationChecker->isGrantedForUser($token->getUser(), $attribute)); } } From c83ae6df9b217e9d405f3a4ce69742d44961d340 Mon Sep 17 00:00:00 2001 From: kor3k Date: Sat, 21 Dec 2024 19:05:36 +0100 Subject: [PATCH 35/52] Sync Security\ExpressionLanguage constructor with parent change typehint array -> iterable --- Authorization/ExpressionLanguage.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Authorization/ExpressionLanguage.php b/Authorization/ExpressionLanguage.php index 846d2cf6..35f32e4d 100644 --- a/Authorization/ExpressionLanguage.php +++ b/Authorization/ExpressionLanguage.php @@ -29,8 +29,12 @@ class_exists(ExpressionLanguageProvider::class); */ class ExpressionLanguage extends BaseExpressionLanguage { - public function __construct(?CacheItemPoolInterface $cache = null, array $providers = []) + public function __construct(?CacheItemPoolInterface $cache = null, iterable $providers = []) { + if (!\is_array($providers)) { + $providers = iterator_to_array($providers, false); + } + // prepend the default provider to let users override it easily array_unshift($providers, new ExpressionLanguageProvider()); From 8f45e009dbb2af198a2cd851447ccddd7c608283 Mon Sep 17 00:00:00 2001 From: Dariusz Ruminski Date: Fri, 10 Jan 2025 15:17:09 +0100 Subject: [PATCH 36/52] chore: PHP CS Fixer fixes --- User/ChainUserChecker.php | 2 +- User/UserCheckerInterface.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/User/ChainUserChecker.php b/User/ChainUserChecker.php index 67fd76b9..eb9ff338 100644 --- a/User/ChainUserChecker.php +++ b/User/ChainUserChecker.php @@ -29,7 +29,7 @@ public function checkPreAuth(UserInterface $user): void } } - public function checkPostAuth(UserInterface $user /*, TokenInterface $token*/): void + public function checkPostAuth(UserInterface $user /* , TokenInterface $token */): void { $token = 1 < \func_num_args() ? func_get_arg(1) : null; diff --git a/User/UserCheckerInterface.php b/User/UserCheckerInterface.php index 2dc748aa..43f1651e 100644 --- a/User/UserCheckerInterface.php +++ b/User/UserCheckerInterface.php @@ -35,5 +35,5 @@ public function checkPreAuth(UserInterface $user): void; * * @throws AccountStatusException */ - public function checkPostAuth(UserInterface $user /*, TokenInterface $token*/): void; + public function checkPostAuth(UserInterface $user /* , TokenInterface $token */): void; } From 3062fe62a6261d1e8a6bea0636d9c03f6b2ba684 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 20 Jan 2025 10:01:54 +0100 Subject: [PATCH 37/52] [Security] Unset token roles when serializing it and user implements EquatableInterface --- Authentication/Token/AbstractToken.php | 26 ++++++++++++++----- ...UserMessageAuthenticationExceptionTest.php | 2 ++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/Authentication/Token/AbstractToken.php b/Authentication/Token/AbstractToken.php index 67d992ce..4a92b606 100644 --- a/Authentication/Token/AbstractToken.php +++ b/Authentication/Token/AbstractToken.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Security\Core\Authentication\Token; +use Symfony\Component\Security\Core\User\EquatableInterface; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Core\User\UserInterface; @@ -23,24 +24,24 @@ abstract class AbstractToken implements TokenInterface, \Serializable { private ?UserInterface $user = null; - private array $roleNames = []; + private array $roleNames; private array $attributes = []; /** * @param string[] $roles An array of roles - * - * @throws \InvalidArgumentException */ public function __construct(array $roles = []) { + $this->roleNames = []; + foreach ($roles as $role) { - $this->roleNames[] = $role; + $this->roleNames[] = (string) $role; } } public function getRoleNames(): array { - return $this->roleNames; + return $this->roleNames ??= self::__construct($this->user->getRoles()) ?? $this->roleNames; } public function getUserIdentifier(): string @@ -82,7 +83,13 @@ public function eraseCredentials(): void */ public function __serialize(): array { - return [$this->user, true, null, $this->attributes, $this->roleNames]; + $data = [$this->user, true, null, $this->attributes]; + + if (!$this->user instanceof EquatableInterface) { + $data[] = $this->roleNames; + } + + return $data; } /** @@ -103,7 +110,12 @@ public function __serialize(): array */ public function __unserialize(array $data): void { - [$user, , , $this->attributes, $this->roleNames] = $data; + [$user, , , $this->attributes] = $data; + + if (\array_key_exists(4, $data)) { + $this->roleNames = $data[4]; + } + $this->user = \is_string($user) ? new InMemoryUser($user, '', $this->roleNames, false) : $user; } diff --git a/Tests/Exception/CustomUserMessageAuthenticationExceptionTest.php b/Tests/Exception/CustomUserMessageAuthenticationExceptionTest.php index 5555ce0b..5a874291 100644 --- a/Tests/Exception/CustomUserMessageAuthenticationExceptionTest.php +++ b/Tests/Exception/CustomUserMessageAuthenticationExceptionTest.php @@ -53,6 +53,7 @@ public function testSharedSerializedData() $exception->setSafeMessage('message', ['token' => $token]); $processed = unserialize(serialize($exception)); + $this->assertSame($token->getRoleNames(), $processed->getToken()->getRoleNames()); $this->assertEquals($token, $processed->getToken()); $this->assertEquals($token, $processed->getMessageData()['token']); $this->assertSame($processed->getToken(), $processed->getMessageData()['token']); @@ -67,6 +68,7 @@ public function testSharedSerializedDataFromChild() $exception->setToken($token); $processed = unserialize(serialize($exception)); + $this->assertSame($token->getRoleNames(), $processed->getToken()->getRoleNames()); $this->assertEquals($token, $processed->childMember); $this->assertEquals($token, $processed->getToken()); $this->assertSame($processed->getToken(), $processed->childMember); From 658e5c856d04ea89cb8bf4963309eb803a3aa108 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 17 Jan 2025 16:58:49 +0100 Subject: [PATCH 38/52] [Security] Don't invalidate the user when the password was not stored in the session --- .../Token/Fixtures/CustomUser.php | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/Tests/Authentication/Token/Fixtures/CustomUser.php b/Tests/Authentication/Token/Fixtures/CustomUser.php index 99302032..c801cc20 100644 --- a/Tests/Authentication/Token/Fixtures/CustomUser.php +++ b/Tests/Authentication/Token/Fixtures/CustomUser.php @@ -1,20 +1,24 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + namespace Symfony\Component\Security\Core\Tests\Authentication\Token\Fixtures; use Symfony\Component\Security\Core\User\UserInterface; final class CustomUser implements UserInterface { - /** @var string */ - private $username; - /** @var array */ - private $roles; - - public function __construct(string $username, array $roles) - { - $this->username = $username; - $this->roles = $roles; + public function __construct( + private string $username, + private array $roles, + ) { } public function getUserIdentifier(): string @@ -27,16 +31,6 @@ public function getRoles(): array return $this->roles; } - public function getPassword(): ?string - { - return null; - } - - public function getSalt(): ?string - { - return null; - } - public function eraseCredentials(): void { } From 1aadc21b0338d67f506b7bfe1b95c095cbb808ad Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Fri, 6 Dec 2024 11:20:22 +0100 Subject: [PATCH 39/52] [Security] Deprecate `UserInterface` & `TokenInterface`'s `eraseCredentials()` --- Authentication/Token/AbstractToken.php | 7 +++++++ Authentication/Token/NullToken.php | 5 +++++ Authentication/Token/TokenInterface.php | 3 +++ CHANGELOG.md | 2 ++ Tests/Authentication/Token/AbstractTokenTest.php | 7 +++++++ Tests/User/InMemoryUserTest.php | 3 +++ User/InMemoryUser.php | 1 + User/UserInterface.php | 16 ++++++++++------ 8 files changed, 38 insertions(+), 6 deletions(-) diff --git a/Authentication/Token/AbstractToken.php b/Authentication/Token/AbstractToken.php index 4a92b606..382d92c8 100644 --- a/Authentication/Token/AbstractToken.php +++ b/Authentication/Token/AbstractToken.php @@ -59,8 +59,15 @@ public function setUser(UserInterface $user): void $this->user = $user; } + /** + * Removes sensitive information from the token. + * + * @deprecated since Symfony 7.3 + */ public function eraseCredentials(): void { + trigger_deprecation('symfony/security-core', '7.3', sprintf('The "%s()" method is deprecated and will be removed in 8.0, use a DTO instead or implement your own erasing logic if needed.', __METHOD__)); + if ($this->getUser() instanceof UserInterface) { $this->getUser()->eraseCredentials(); } diff --git a/Authentication/Token/NullToken.php b/Authentication/Token/NullToken.php index 9c2e4892..1c60357c 100644 --- a/Authentication/Token/NullToken.php +++ b/Authentication/Token/NullToken.php @@ -43,6 +43,11 @@ public function getUserIdentifier(): string return ''; } + /** + * Removes sensitive information from the token. + * + * @deprecated since Symfony 7.3 + */ public function eraseCredentials(): void { } diff --git a/Authentication/Token/TokenInterface.php b/Authentication/Token/TokenInterface.php index 1e67b1e5..8fb58c16 100644 --- a/Authentication/Token/TokenInterface.php +++ b/Authentication/Token/TokenInterface.php @@ -56,6 +56,9 @@ public function setUser(UserInterface $user): void; /** * Removes sensitive information from the token. + * + * @deprecated since Symfony 7.3, use a dedicated DTO instead or implement your + * own erasing logic instead */ public function eraseCredentials(): void; diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cc738ce..b3d13372 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ CHANGELOG * Add `UserAuthorizationChecker::isGrantedForUser()` to test user authorization without relying on the session. For example, users not currently logged in, or while processing a message from a message queue. * Add `OfflineTokenInterface` to mark tokens that do not represent the currently logged-in user + * Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`, + use a dedicated DTO or erase credentials on your own e.g. upon `AuthenticationTokenCreatedEvent` instead 7.2 --- diff --git a/Tests/Authentication/Token/AbstractTokenTest.php b/Tests/Authentication/Token/AbstractTokenTest.php index cc1357a1..33b65206 100644 --- a/Tests/Authentication/Token/AbstractTokenTest.php +++ b/Tests/Authentication/Token/AbstractTokenTest.php @@ -12,12 +12,15 @@ namespace Symfony\Component\Security\Core\Tests\Authentication\Token; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\Authentication\Token\AbstractToken; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Core\User\UserInterface; class AbstractTokenTest extends TestCase { + use ExpectDeprecationTrait; + /** * @dataProvider provideUsers */ @@ -33,6 +36,9 @@ public static function provideUsers() yield [new InMemoryUser('fabien', null), 'fabien']; } + /** + * @group legacy + */ public function testEraseCredentials() { $token = new ConcreteToken(['ROLE_FOO']); @@ -40,6 +46,7 @@ public function testEraseCredentials() $user = $this->createMock(UserInterface::class); $user->expects($this->once())->method('eraseCredentials'); $token->setUser($user); + $this->expectDeprecation('The Symfony\Component\Security\Core\User\UserInterface::eraseCredentials method is deprecated (since Symfony 7.3, use a dedicated DTO instead or implement your own erasing logic instead).'); $token->eraseCredentials(); } diff --git a/Tests/User/InMemoryUserTest.php b/Tests/User/InMemoryUserTest.php index 0e64bce5..4537b1f6 100644 --- a/Tests/User/InMemoryUserTest.php +++ b/Tests/User/InMemoryUserTest.php @@ -53,6 +53,9 @@ public function testIsEnabled() $this->assertFalse($user->isEnabled()); } + /** + * @group legacy + */ public function testEraseCredentials() { $user = new InMemoryUser('fabien', 'superpass'); diff --git a/User/InMemoryUser.php b/User/InMemoryUser.php index b14bc077..bfaae97f 100644 --- a/User/InMemoryUser.php +++ b/User/InMemoryUser.php @@ -76,6 +76,7 @@ public function isEnabled(): bool public function eraseCredentials(): void { + trigger_deprecation('symfony/security-core', '7.3', sprintf('The "%s()" method is deprecated and will be removed in 8.0, use a DTO instead or implement your own erasing logic if needed.', __METHOD__)); } public function isEqualTo(UserInterface $user): bool diff --git a/User/UserInterface.php b/User/UserInterface.php index e6078399..9bee95c9 100644 --- a/User/UserInterface.php +++ b/User/UserInterface.php @@ -46,18 +46,22 @@ interface UserInterface */ public function getRoles(): array; + /** + * Returns the identifier for this user (e.g. username or email address). + * + * @return non-empty-string + */ + public function getUserIdentifier(): string; + /** * Removes sensitive data from the user. * * This is important if, at any given point, sensitive information like * the plain-text password is stored on this object. + * + * @deprecated since Symfony 7.3, use a dedicated DTO instead or implement your + * own erasing logic instead */ public function eraseCredentials(): void; - /** - * Returns the identifier for this user (e.g. username or email address). - * - * @return non-empty-string - */ - public function getUserIdentifier(): string; } From f5f6b03bafb27a15ac63f5a0eb25a15d535747d5 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 3 Feb 2025 18:35:30 +0100 Subject: [PATCH 40/52] [Security] Improve BC-layer to deprecate eraseCredentials methods --- Authentication/Token/AbstractToken.php | 4 ++-- Authentication/Token/NullToken.php | 6 ++++-- Authentication/Token/TokenInterface.php | 6 ++++-- CHANGELOG.md | 2 +- .../AuthenticationTrustResolverTest.php | 1 + .../Token/AbstractTokenTest.php | 4 +++- .../Token/Fixtures/CustomUser.php | 6 ++++++ Tests/User/InMemoryUserTest.php | 4 ++++ User/InMemoryUser.php | 8 +++++++- User/OidcUser.php | 7 +++++++ User/PasswordAuthenticatedUserInterface.php | 3 +++ User/UserInterface.php | 19 ++++++++++--------- 12 files changed, 52 insertions(+), 18 deletions(-) diff --git a/Authentication/Token/AbstractToken.php b/Authentication/Token/AbstractToken.php index 382d92c8..b2e18a29 100644 --- a/Authentication/Token/AbstractToken.php +++ b/Authentication/Token/AbstractToken.php @@ -62,11 +62,11 @@ public function setUser(UserInterface $user): void /** * Removes sensitive information from the token. * - * @deprecated since Symfony 7.3 + * @deprecated since Symfony 7.3, erase credentials using the "__serialize()" method instead */ public function eraseCredentials(): void { - trigger_deprecation('symfony/security-core', '7.3', sprintf('The "%s()" method is deprecated and will be removed in 8.0, use a DTO instead or implement your own erasing logic if needed.', __METHOD__)); + trigger_deprecation('symfony/security-core', '7.3', \sprintf('The "%s::eraseCredentials()" method is deprecated and will be removed in 8.0, erase credentials using the "__serialize()" method instead.', TokenInterface::class)); if ($this->getUser() instanceof UserInterface) { $this->getUser()->eraseCredentials(); diff --git a/Authentication/Token/NullToken.php b/Authentication/Token/NullToken.php index 1c60357c..cb2bc0fd 100644 --- a/Authentication/Token/NullToken.php +++ b/Authentication/Token/NullToken.php @@ -44,12 +44,14 @@ public function getUserIdentifier(): string } /** - * Removes sensitive information from the token. - * * @deprecated since Symfony 7.3 */ + #[\Deprecated(since: 'symfony/security-core 7.3')] public function eraseCredentials(): void { + if (\PHP_VERSION_ID < 80400) { + @trigger_error(\sprintf('Method %s::eraseCredentials() is deprecated since symfony/security-core 7.3', self::class), \E_USER_DEPRECATED); + } } public function getAttributes(): array diff --git a/Authentication/Token/TokenInterface.php b/Authentication/Token/TokenInterface.php index 8fb58c16..c658e38b 100644 --- a/Authentication/Token/TokenInterface.php +++ b/Authentication/Token/TokenInterface.php @@ -16,6 +16,9 @@ /** * TokenInterface is the interface for the user authentication information. * + * The __serialize/__unserialize() magic methods can be implemented on the token + * class to prevent sensitive credentials from being put in the session storage. + * * @author Fabien Potencier * @author Johannes M. Schmitt */ @@ -57,8 +60,7 @@ public function setUser(UserInterface $user): void; /** * Removes sensitive information from the token. * - * @deprecated since Symfony 7.3, use a dedicated DTO instead or implement your - * own erasing logic instead + * @deprecated since Symfony 7.3; erase credentials using the "__serialize()" method instead */ public function eraseCredentials(): void; diff --git a/CHANGELOG.md b/CHANGELOG.md index b3d13372..290aa8b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ CHANGELOG For example, users not currently logged in, or while processing a message from a message queue. * Add `OfflineTokenInterface` to mark tokens that do not represent the currently logged-in user * Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`, - use a dedicated DTO or erase credentials on your own e.g. upon `AuthenticationTokenCreatedEvent` instead + erase credentials e.g. using `__serialize()` instead 7.2 --- diff --git a/Tests/Authentication/AuthenticationTrustResolverTest.php b/Tests/Authentication/AuthenticationTrustResolverTest.php index fc559983..c657b31e 100644 --- a/Tests/Authentication/AuthenticationTrustResolverTest.php +++ b/Tests/Authentication/AuthenticationTrustResolverTest.php @@ -119,6 +119,7 @@ public function getUserIdentifier(): string { } + #[\Deprecated] public function eraseCredentials(): void { } diff --git a/Tests/Authentication/Token/AbstractTokenTest.php b/Tests/Authentication/Token/AbstractTokenTest.php index 33b65206..ef3d380c 100644 --- a/Tests/Authentication/Token/AbstractTokenTest.php +++ b/Tests/Authentication/Token/AbstractTokenTest.php @@ -14,6 +14,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\Authentication\Token\AbstractToken; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Core\User\UserInterface; @@ -46,7 +47,8 @@ public function testEraseCredentials() $user = $this->createMock(UserInterface::class); $user->expects($this->once())->method('eraseCredentials'); $token->setUser($user); - $this->expectDeprecation('The Symfony\Component\Security\Core\User\UserInterface::eraseCredentials method is deprecated (since Symfony 7.3, use a dedicated DTO instead or implement your own erasing logic instead).'); + + $this->expectDeprecation(\sprintf('Since symfony/security-core 7.3: The "%s::eraseCredentials()" method is deprecated and will be removed in 8.0, erase credentials using the "__serialize()" method instead.', TokenInterface::class)); $token->eraseCredentials(); } diff --git a/Tests/Authentication/Token/Fixtures/CustomUser.php b/Tests/Authentication/Token/Fixtures/CustomUser.php index c801cc20..d4f91de1 100644 --- a/Tests/Authentication/Token/Fixtures/CustomUser.php +++ b/Tests/Authentication/Token/Fixtures/CustomUser.php @@ -31,6 +31,12 @@ public function getRoles(): array return $this->roles; } + public function getPassword(): ?string + { + return null; + } + + #[\Deprecated] public function eraseCredentials(): void { } diff --git a/Tests/User/InMemoryUserTest.php b/Tests/User/InMemoryUserTest.php index 4537b1f6..501bf742 100644 --- a/Tests/User/InMemoryUserTest.php +++ b/Tests/User/InMemoryUserTest.php @@ -12,11 +12,14 @@ namespace Symfony\Component\Security\Core\Tests\User; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Core\User\UserInterface; class InMemoryUserTest extends TestCase { + use ExpectDeprecationTrait; + public function testConstructorException() { $this->expectException(\InvalidArgumentException::class); @@ -59,6 +62,7 @@ public function testIsEnabled() public function testEraseCredentials() { $user = new InMemoryUser('fabien', 'superpass'); + $this->expectDeprecation(\sprintf('%sMethod %s::eraseCredentials() is deprecated since symfony/security-core 7.3', \PHP_VERSION_ID >= 80400 ? 'Unsilenced deprecation: ' : '', InMemoryUser::class)); $user->eraseCredentials(); $this->assertEquals('superpass', $user->getPassword()); } diff --git a/User/InMemoryUser.php b/User/InMemoryUser.php index bfaae97f..7bed183a 100644 --- a/User/InMemoryUser.php +++ b/User/InMemoryUser.php @@ -74,9 +74,15 @@ public function isEnabled(): bool return $this->enabled; } + /** + * @deprecated since Symfony 7.3 + */ + #[\Deprecated(since: 'symfony/security-core 7.3')] public function eraseCredentials(): void { - trigger_deprecation('symfony/security-core', '7.3', sprintf('The "%s()" method is deprecated and will be removed in 8.0, use a DTO instead or implement your own erasing logic if needed.', __METHOD__)); + if (\PHP_VERSION_ID < 80400) { + @trigger_error(\sprintf('Method %s::eraseCredentials() is deprecated since symfony/security-core 7.3', self::class), \E_USER_DEPRECATED); + } } public function isEqualTo(UserInterface $user): bool diff --git a/User/OidcUser.php b/User/OidcUser.php index bcce363f..df59c5f7 100644 --- a/User/OidcUser.php +++ b/User/OidcUser.php @@ -71,8 +71,15 @@ public function getUserIdentifier(): string return (string) ($this->userIdentifier ?? $this->getSub()); } + /** + * @deprecated since Symfony 7.3 + */ + #[\Deprecated(since: 'symfony/security-core 7.3')] public function eraseCredentials(): void { + if (\PHP_VERSION_ID < 80400) { + @trigger_error(\sprintf('Method %s::eraseCredentials() is deprecated since symfony/security-core 7.3', self::class), \E_USER_DEPRECATED); + } } public function getSub(): ?string diff --git a/User/PasswordAuthenticatedUserInterface.php b/User/PasswordAuthenticatedUserInterface.php index 478c9e38..900963d1 100644 --- a/User/PasswordAuthenticatedUserInterface.php +++ b/User/PasswordAuthenticatedUserInterface.php @@ -23,6 +23,9 @@ interface PasswordAuthenticatedUserInterface * Returns the hashed password used to authenticate the user. * * Usually on authentication, a plain-text password will be compared to this value. + * + * The __serialize/__unserialize() magic methods can be implemented on the user + * class to prevent hashed passwords from being put in the session storage. */ public function getPassword(): ?string; } diff --git a/User/UserInterface.php b/User/UserInterface.php index 9bee95c9..12052121 100644 --- a/User/UserInterface.php +++ b/User/UserInterface.php @@ -24,6 +24,9 @@ * this interface. Objects that implement this interface are created and * loaded by different objects that implement UserProviderInterface. * + * The __serialize/__unserialize() magic methods can be implemented on the user + * class to prevent sensitive credentials from being put in the session storage. + * * @see UserProviderInterface * * @author Fabien Potencier @@ -46,22 +49,20 @@ interface UserInterface */ public function getRoles(): array; - /** - * Returns the identifier for this user (e.g. username or email address). - * - * @return non-empty-string - */ - public function getUserIdentifier(): string; - /** * Removes sensitive data from the user. * * This is important if, at any given point, sensitive information like * the plain-text password is stored on this object. * - * @deprecated since Symfony 7.3, use a dedicated DTO instead or implement your - * own erasing logic instead + * @deprecated since Symfony 7.3, erase credentials using the "__serialize()" method instead */ public function eraseCredentials(): void; + /** + * Returns the identifier for this user (e.g. username or email address). + * + * @return non-empty-string + */ + public function getUserIdentifier(): string; } From 32051818e61ff8b37bf8b2937fbbf00949c1e1c4 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 20 Jan 2025 16:22:38 +0100 Subject: [PATCH 41/52] [Security] Support hashing the hashed password using crc32c when putting the user in the session --- User/PasswordAuthenticatedUserInterface.php | 23 ++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/User/PasswordAuthenticatedUserInterface.php b/User/PasswordAuthenticatedUserInterface.php index 900963d1..01613ec2 100644 --- a/User/PasswordAuthenticatedUserInterface.php +++ b/User/PasswordAuthenticatedUserInterface.php @@ -14,6 +14,26 @@ /** * For users that can be authenticated using a password. * + * The __serialize/__unserialize() magic methods can be implemented on the user + * class to prevent hashed passwords from being put in the session storage. + * If the password is not stored at all in the session, getPassword() should + * return null after unserialization, and then, changing the user's password + * won't invalidate its sessions. + * In order to invalidate the user sessions while not storing the password hash + * in the session, it's also possible to hash the password hash before + * serializing it; crc32c is the only algorithm supported. + * For example: + * + * public function __serialize(): array + * { + * $data = (array) $this; + * $data["\0".self::class."\0password"] = hash('crc32c', $this->password); + * + * return $data; + * } + * + * Implement EquatableInteface if you need another logic. + * * @author Robin Chalas * @author Wouter de Jong */ @@ -23,9 +43,6 @@ interface PasswordAuthenticatedUserInterface * Returns the hashed password used to authenticate the user. * * Usually on authentication, a plain-text password will be compared to this value. - * - * The __serialize/__unserialize() magic methods can be implemented on the user - * class to prevent hashed passwords from being put in the session storage. */ public function getPassword(): ?string; } From 5e5c2181d0deae0e5b09e4fe8b42690618db8e64 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 27 Aug 2024 19:49:01 +0200 Subject: [PATCH 42/52] [Security] Add ability for voters to explain their vote --- Authorization/AccessDecision.php | 56 +++++++++++++ Authorization/AccessDecisionManager.php | 40 ++++++++-- .../AccessDecisionManagerInterface.php | 7 +- Authorization/AuthorizationChecker.php | 12 ++- .../AuthorizationCheckerInterface.php | 5 +- .../TraceableAccessDecisionManager.php | 79 +++++++++---------- Authorization/UserAuthorizationChecker.php | 4 +- .../UserAuthorizationCheckerInterface.php | 5 +- Authorization/Voter/AuthenticatedVoter.php | 33 ++++++-- Authorization/Voter/ExpressionVoter.php | 24 +++++- Authorization/Voter/RoleVoter.php | 17 +++- Authorization/Voter/TraceableVoter.php | 6 +- Authorization/Voter/Vote.php | 35 ++++++++ Authorization/Voter/Voter.php | 20 +++-- Authorization/Voter/VoterInterface.php | 7 +- CHANGELOG.md | 1 + Event/VoteEvent.php | 6 ++ Exception/AccessDeniedException.php | 12 +++ Test/AccessDecisionStrategyTestCase.php | 3 +- .../TraceableAccessDecisionManagerTest.php | 32 ++++---- Tests/Authorization/Voter/VoterTest.php | 7 +- Tests/Fixtures/DummyVoter.php | 3 +- 22 files changed, 311 insertions(+), 103 deletions(-) create mode 100644 Authorization/AccessDecision.php create mode 100644 Authorization/Voter/Vote.php diff --git a/Authorization/AccessDecision.php b/Authorization/AccessDecision.php new file mode 100644 index 00000000..d5465a1c --- /dev/null +++ b/Authorization/AccessDecision.php @@ -0,0 +1,56 @@ + + * + * 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; + +use Symfony\Component\Security\Core\Authorization\Voter\Vote; +use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; + +/** + * Contains the access verdict and all the related votes. + * + * @author Dany Maillard + * @author Roman JOLY + * @author Nicolas Grekas + */ +class AccessDecision +{ + /** + * @var class-string|string|null + */ + public ?string $strategy = null; + + public bool $isGranted; + + /** + * @var Vote[] + */ + public array $votes = []; + + public function getMessage(): string + { + $message = $this->isGranted ? 'Access Granted.' : 'Access Denied.'; + $access = $this->isGranted ? VoterInterface::ACCESS_GRANTED : VoterInterface::ACCESS_DENIED; + + if ($this->votes) { + foreach ($this->votes as $vote) { + if ($vote->result !== $access) { + continue; + } + foreach ($vote->reasons as $reason) { + $message .= ' '.$reason; + } + } + } + + return $message; + } +} diff --git a/Authorization/AccessDecisionManager.php b/Authorization/AccessDecisionManager.php index 3e42c4bf..fd435386 100644 --- a/Authorization/AccessDecisionManager.php +++ b/Authorization/AccessDecisionManager.php @@ -15,6 +15,8 @@ use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface; use Symfony\Component\Security\Core\Authorization\Strategy\AffirmativeStrategy; use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface; +use Symfony\Component\Security\Core\Authorization\Voter\TraceableVoter; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Exception\InvalidArgumentException; @@ -35,6 +37,7 @@ final class AccessDecisionManager implements AccessDecisionManagerInterface private array $votersCacheAttributes = []; private array $votersCacheObject = []; private AccessDecisionStrategyInterface $strategy; + private array $accessDecisionStack = []; /** * @param iterable $voters An array or an iterator of VoterInterface instances @@ -49,35 +52,56 @@ public function __construct( /** * @param bool $allowMultipleAttributes Whether to allow passing multiple values to the $attributes array */ - public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false): bool + public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool|AccessDecision|null $accessDecision = null, bool $allowMultipleAttributes = false): bool { + if (\is_bool($accessDecision)) { + $allowMultipleAttributes = $accessDecision; + $accessDecision = null; + } + // Special case for AccessListener, do not remove the right side of the condition before 6.0 if (\count($attributes) > 1 && !$allowMultipleAttributes) { throw new InvalidArgumentException(\sprintf('Passing more than one Security attribute to "%s()" is not supported.', __METHOD__)); } - return $this->strategy->decide( - $this->collectResults($token, $attributes, $object) - ); + $accessDecision ??= end($this->accessDecisionStack) ?: new AccessDecision(); + $this->accessDecisionStack[] = $accessDecision; + + $accessDecision->strategy = $this->strategy instanceof \Stringable ? $this->strategy : get_debug_type($this->strategy); + + try { + return $accessDecision->isGranted = $this->strategy->decide( + $this->collectResults($token, $attributes, $object, $accessDecision) + ); + } finally { + array_pop($this->accessDecisionStack); + } } /** - * @return \Traversable + * @return \Traversable */ - private function collectResults(TokenInterface $token, array $attributes, mixed $object): \Traversable + private function collectResults(TokenInterface $token, array $attributes, mixed $object, AccessDecision $accessDecision): \Traversable { foreach ($this->getVoters($attributes, $object) as $voter) { - $result = $voter->vote($token, $object, $attributes); + $vote = new Vote(); + $result = $voter->vote($token, $object, $attributes, $vote); + if (!\is_int($result) || !(self::VALID_VOTES[$result] ?? false)) { throw new \LogicException(\sprintf('"%s::vote()" must return one of "%s" constants ("ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN"), "%s" returned.', get_debug_type($voter), VoterInterface::class, var_export($result, true))); } + $voter = $voter instanceof TraceableVoter ? $voter->getDecoratedVoter() : $voter; + $vote->voter = $voter instanceof \Stringable ? $voter : get_debug_type($voter); + $vote->result = $result; + $accessDecision->votes[] = $vote; + yield $result; } } /** - * @return iterable + * @return iterable */ private function getVoters(array $attributes, $object = null): iterable { diff --git a/Authorization/AccessDecisionManagerInterface.php b/Authorization/AccessDecisionManagerInterface.php index f25c7e1b..cb4a3310 100644 --- a/Authorization/AccessDecisionManagerInterface.php +++ b/Authorization/AccessDecisionManagerInterface.php @@ -23,8 +23,9 @@ interface AccessDecisionManagerInterface /** * Decides whether the access is possible or not. * - * @param array $attributes An array of attributes associated with the method being invoked - * @param mixed $object The object to secure + * @param array $attributes An array of attributes associated with the method being invoked + * @param mixed $object The object to secure + * @param AccessDecision|null $accessDecision Should be used to explain the decision */ - public function decide(TokenInterface $token, array $attributes, mixed $object = null): bool; + public function decide(TokenInterface $token, array $attributes, mixed $object = null/* , ?AccessDecision $accessDecision = null */): bool; } diff --git a/Authorization/AuthorizationChecker.php b/Authorization/AuthorizationChecker.php index c748697c..3960f2be 100644 --- a/Authorization/AuthorizationChecker.php +++ b/Authorization/AuthorizationChecker.php @@ -24,20 +24,28 @@ */ class AuthorizationChecker implements AuthorizationCheckerInterface { + private array $accessDecisionStack = []; + public function __construct( private TokenStorageInterface $tokenStorage, private AccessDecisionManagerInterface $accessDecisionManager, ) { } - final public function isGranted(mixed $attribute, mixed $subject = null): bool + final public function isGranted(mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool { $token = $this->tokenStorage->getToken(); if (!$token || !$token->getUser()) { $token = new NullToken(); } + $accessDecision ??= end($this->accessDecisionStack) ?: new AccessDecision(); + $this->accessDecisionStack[] = $accessDecision; - return $this->accessDecisionManager->decide($token, [$attribute], $subject); + try { + return $accessDecision->isGranted = $this->accessDecisionManager->decide($token, [$attribute], $subject, $accessDecision); + } finally { + array_pop($this->accessDecisionStack); + } } } diff --git a/Authorization/AuthorizationCheckerInterface.php b/Authorization/AuthorizationCheckerInterface.php index 6f5a6022..7c673dfc 100644 --- a/Authorization/AuthorizationCheckerInterface.php +++ b/Authorization/AuthorizationCheckerInterface.php @@ -21,7 +21,8 @@ interface AuthorizationCheckerInterface /** * Checks if the attribute is granted against the current authentication token and optionally supplied subject. * - * @param mixed $attribute A single attribute to vote on (can be of any type, string and instance of Expression are supported by the core) + * @param mixed $attribute A single attribute to vote on (can be of any type, string and instance of Expression are supported by the core) + * @param AccessDecision|null $accessDecision Should be used to explain the decision */ - public function isGranted(mixed $attribute, mixed $subject = null): bool; + public function isGranted(mixed $attribute, mixed $subject = null/* , ?AccessDecision $accessDecision = null */): bool; } diff --git a/Authorization/TraceableAccessDecisionManager.php b/Authorization/TraceableAccessDecisionManager.php index 0b82eb3a..a03e2d0c 100644 --- a/Authorization/TraceableAccessDecisionManager.php +++ b/Authorization/TraceableAccessDecisionManager.php @@ -12,7 +12,6 @@ namespace Symfony\Component\Security\Core\Authorization; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; -use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -25,77 +24,68 @@ */ class TraceableAccessDecisionManager implements AccessDecisionManagerInterface { - private ?AccessDecisionStrategyInterface $strategy = null; - /** @var iterable */ - private iterable $voters = []; + private ?string $strategy = null; + /** @var array */ + private array $voters = []; private array $decisionLog = []; // All decision logs private array $currentLog = []; // Logs being filled in + private array $accessDecisionStack = []; public function __construct( private AccessDecisionManagerInterface $manager, ) { - // The strategy and voters are stored in a private properties of the decorated service - if (property_exists($manager, 'strategy')) { - $reflection = new \ReflectionProperty($manager::class, 'strategy'); - $this->strategy = $reflection->getValue($manager); - } - if (property_exists($manager, 'voters')) { - $reflection = new \ReflectionProperty($manager::class, 'voters'); - $this->voters = $reflection->getValue($manager); - } } - public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false): bool + public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool|AccessDecision|null $accessDecision = null, bool $allowMultipleAttributes = false): bool { - $currentDecisionLog = [ + if (\is_bool($accessDecision)) { + $allowMultipleAttributes = $accessDecision; + $accessDecision = null; + } + + // Using a stack since decide can be called by voters + $this->currentLog[] = [ 'attributes' => $attributes, 'object' => $object, 'voterDetails' => [], ]; - $this->currentLog[] = &$currentDecisionLog; - - $result = $this->manager->decide($token, $attributes, $object, $allowMultipleAttributes); - - $currentDecisionLog['result'] = $result; - - $this->decisionLog[] = array_pop($this->currentLog); // Using a stack since decide can be called by voters - - return $result; + $accessDecision ??= end($this->accessDecisionStack) ?: new AccessDecision(); + $this->accessDecisionStack[] = $accessDecision; + + try { + return $accessDecision->isGranted = $this->manager->decide($token, $attributes, $object, $accessDecision); + } finally { + $this->strategy = $accessDecision->strategy; + $currentLog = array_pop($this->currentLog); + if (isset($accessDecision->isGranted)) { + $currentLog['result'] = $accessDecision->isGranted; + } + $this->decisionLog[] = $currentLog; + } } - /** - * Adds voter vote and class to the voter details. - * - * @param array $attributes attributes used for the vote - * @param int $vote vote of the voter - */ - public function addVoterVote(VoterInterface $voter, array $attributes, int $vote): void + public function addVoterVote(VoterInterface $voter, array $attributes, int $vote, array $reasons = []): void { $currentLogIndex = \count($this->currentLog) - 1; $this->currentLog[$currentLogIndex]['voterDetails'][] = [ 'voter' => $voter, 'attributes' => $attributes, 'vote' => $vote, + 'reasons' => $reasons, ]; + $this->voters[$voter::class] = $voter; } public function getStrategy(): string { - if (null === $this->strategy) { - return '-'; - } - if ($this->strategy instanceof \Stringable) { - return (string) $this->strategy; - } - - return get_debug_type($this->strategy); + return $this->strategy ?? '-'; } /** - * @return iterable + * @return array */ - public function getVoters(): iterable + public function getVoters(): array { return $this->voters; } @@ -104,4 +94,11 @@ public function getDecisionLog(): array { return $this->decisionLog; } + + public function reset(): void + { + $this->strategy = null; + $this->voters = []; + $this->decisionLog = []; + } } diff --git a/Authorization/UserAuthorizationChecker.php b/Authorization/UserAuthorizationChecker.php index f5ba7b88..f515e5cb 100644 --- a/Authorization/UserAuthorizationChecker.php +++ b/Authorization/UserAuthorizationChecker.php @@ -24,8 +24,8 @@ public function __construct( ) { } - public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null): bool + public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool { - return $this->accessDecisionManager->decide(new UserAuthorizationCheckerToken($user), [$attribute], $subject); + return $this->accessDecisionManager->decide(new UserAuthorizationCheckerToken($user), [$attribute], $subject, $accessDecision); } } diff --git a/Authorization/UserAuthorizationCheckerInterface.php b/Authorization/UserAuthorizationCheckerInterface.php index 3335e6fd..15e5b4d4 100644 --- a/Authorization/UserAuthorizationCheckerInterface.php +++ b/Authorization/UserAuthorizationCheckerInterface.php @@ -23,7 +23,8 @@ interface UserAuthorizationCheckerInterface /** * Checks if the attribute is granted against the user and optionally supplied subject. * - * @param mixed $attribute A single attribute to vote on (can be of any type, string and instance of Expression are supported by the core) + * @param mixed $attribute A single attribute to vote on (can be of any type, string and instance of Expression are supported by the core) + * @param AccessDecision|null $accessDecision Should be used to explain the decision */ - public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null): bool; + public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool; } diff --git a/Authorization/Voter/AuthenticatedVoter.php b/Authorization/Voter/AuthenticatedVoter.php index a073f616..1403aaaa 100644 --- a/Authorization/Voter/AuthenticatedVoter.php +++ b/Authorization/Voter/AuthenticatedVoter.php @@ -40,9 +40,17 @@ public function __construct( ) { } - public function vote(TokenInterface $token, mixed $subject, array $attributes): int + /** + * @param Vote|null $vote Should be used to explain the vote + */ + public function vote(TokenInterface $token, mixed $subject, array $attributes/* , ?Vote $vote = null */): int { + $vote = 3 < \func_num_args() ? func_get_arg(3) : new Vote(); + $vote ??= new Vote(); + if ($attributes === [self::PUBLIC_ACCESS]) { + $vote->reasons[] = 'Access is public.'; + return VoterInterface::ACCESS_GRANTED; } @@ -62,30 +70,45 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes): $result = VoterInterface::ACCESS_DENIED; - if (self::IS_AUTHENTICATED_FULLY === $attribute - && $this->authenticationTrustResolver->isFullFledged($token)) { + if ((self::IS_AUTHENTICATED_FULLY === $attribute || self::IS_AUTHENTICATED_REMEMBERED === $attribute) + && $this->authenticationTrustResolver->isFullFledged($token) + ) { + $vote->reasons[] = 'The user is fully authenticated.'; + return VoterInterface::ACCESS_GRANTED; } if (self::IS_AUTHENTICATED_REMEMBERED === $attribute - && ($this->authenticationTrustResolver->isRememberMe($token) - || $this->authenticationTrustResolver->isFullFledged($token))) { + && $this->authenticationTrustResolver->isRememberMe($token) + ) { + $vote->reasons[] = 'The user is remembered.'; + return VoterInterface::ACCESS_GRANTED; } if (self::IS_AUTHENTICATED === $attribute && $this->authenticationTrustResolver->isAuthenticated($token)) { + $vote->reasons[] = 'The user is authenticated.'; + return VoterInterface::ACCESS_GRANTED; } if (self::IS_REMEMBERED === $attribute && $this->authenticationTrustResolver->isRememberMe($token)) { + $vote->reasons[] = 'The user is remembered.'; + return VoterInterface::ACCESS_GRANTED; } if (self::IS_IMPERSONATOR === $attribute && $token instanceof SwitchUserToken) { + $vote->reasons[] = 'The user is impersonating another user.'; + return VoterInterface::ACCESS_GRANTED; } } + if (VoterInterface::ACCESS_DENIED === $result) { + $vote->reasons[] = 'The user is not appropriately authenticated.'; + } + return $result; } diff --git a/Authorization/Voter/ExpressionVoter.php b/Authorization/Voter/ExpressionVoter.php index bab32830..35d727a8 100644 --- a/Authorization/Voter/ExpressionVoter.php +++ b/Authorization/Voter/ExpressionVoter.php @@ -28,7 +28,7 @@ class ExpressionVoter implements CacheableVoterInterface { public function __construct( private ExpressionLanguage $expressionLanguage, - private AuthenticationTrustResolverInterface $trustResolver, + private ?AuthenticationTrustResolverInterface $trustResolver, private AuthorizationCheckerInterface $authChecker, private ?RoleHierarchyInterface $roleHierarchy = null, ) { @@ -44,10 +44,16 @@ public function supportsType(string $subjectType): bool return true; } - public function vote(TokenInterface $token, mixed $subject, array $attributes): int + /** + * @param Vote|null $vote Should be used to explain the vote + */ + public function vote(TokenInterface $token, mixed $subject, array $attributes/* , ?Vote $vote = null */): int { + $vote = 3 < \func_num_args() ? func_get_arg(3) : new Vote(); + $vote ??= new Vote(); $result = VoterInterface::ACCESS_ABSTAIN; $variables = null; + $failingExpressions = []; foreach ($attributes as $attribute) { if (!$attribute instanceof Expression) { continue; @@ -56,9 +62,18 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes): $variables ??= $this->getVariables($token, $subject); $result = VoterInterface::ACCESS_DENIED; + if ($this->expressionLanguage->evaluate($attribute, $variables)) { + $vote->reasons[] = \sprintf('Expression (%s) is true.', $attribute); + return VoterInterface::ACCESS_GRANTED; } + + $failingExpressions[] = $attribute; + } + + if ($failingExpressions) { + $vote->reasons[] = \sprintf('Expression (%s) is false.', implode(') || (', $failingExpressions)); } return $result; @@ -78,10 +93,13 @@ private function getVariables(TokenInterface $token, mixed $subject): array 'object' => $subject, 'subject' => $subject, 'role_names' => $roleNames, - 'trust_resolver' => $this->trustResolver, 'auth_checker' => $this->authChecker, ]; + if ($this->trustResolver) { + $variables['trust_resolver'] = $this->trustResolver; + } + // this is mainly to propose a better experience when the expression is used // in an access control rule, as the developer does not know that it's going // to be handled by this voter diff --git a/Authorization/Voter/RoleVoter.php b/Authorization/Voter/RoleVoter.php index 3c65fb63..46c08d15 100644 --- a/Authorization/Voter/RoleVoter.php +++ b/Authorization/Voter/RoleVoter.php @@ -25,10 +25,16 @@ public function __construct( ) { } - public function vote(TokenInterface $token, mixed $subject, array $attributes): int + /** + * @param Vote|null $vote Should be used to explain the vote + */ + public function vote(TokenInterface $token, mixed $subject, array $attributes/* , ?Vote $vote = null */): int { + $vote = 3 < \func_num_args() ? func_get_arg(3) : new Vote(); + $vote ??= new Vote(); $result = VoterInterface::ACCESS_ABSTAIN; $roles = $this->extractRoles($token); + $missingRoles = []; foreach ($attributes as $attribute) { if (!\is_string($attribute) || !str_starts_with($attribute, $this->prefix)) { @@ -36,9 +42,18 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes): } $result = VoterInterface::ACCESS_DENIED; + if (\in_array($attribute, $roles, true)) { + $vote->reasons[] = \sprintf('The user has %s.', $attribute); + return VoterInterface::ACCESS_GRANTED; } + + $missingRoles[] = $attribute; + } + + if (VoterInterface::ACCESS_DENIED === $result) { + $vote->reasons[] = \sprintf('The user doesn\'t have%s %s.', 1 < \count($missingRoles) ? ' any of' : '', implode(', ', $missingRoles)); } return $result; diff --git a/Authorization/Voter/TraceableVoter.php b/Authorization/Voter/TraceableVoter.php index 1abc7c70..47572797 100644 --- a/Authorization/Voter/TraceableVoter.php +++ b/Authorization/Voter/TraceableVoter.php @@ -30,11 +30,11 @@ public function __construct( ) { } - public function vote(TokenInterface $token, mixed $subject, array $attributes): int + public function vote(TokenInterface $token, mixed $subject, array $attributes, ?Vote $vote = null): int { - $result = $this->voter->vote($token, $subject, $attributes); + $result = $this->voter->vote($token, $subject, $attributes, $vote ??= new Vote()); - $this->eventDispatcher->dispatch(new VoteEvent($this->voter, $subject, $attributes, $result), 'debug.security.authorization.vote'); + $this->eventDispatcher->dispatch(new VoteEvent($this->voter, $subject, $attributes, $result, $vote->reasons), 'debug.security.authorization.vote'); return $result; } diff --git a/Authorization/Voter/Vote.php b/Authorization/Voter/Vote.php new file mode 100644 index 00000000..e933c57d --- /dev/null +++ b/Authorization/Voter/Vote.php @@ -0,0 +1,35 @@ + + * + * 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; + +class Vote +{ + /** + * @var class-string|string + */ + public string $voter; + + /** + * @var VoterInterface::ACCESS_* + */ + public int $result; + + /** + * @var list + */ + public array $reasons = []; + + public function addReason(string $reason): void + { + $this->reasons[] = $reason; + } +} diff --git a/Authorization/Voter/Voter.php b/Authorization/Voter/Voter.php index 1f76a42e..3d7fd9e2 100644 --- a/Authorization/Voter/Voter.php +++ b/Authorization/Voter/Voter.php @@ -24,10 +24,15 @@ */ abstract class Voter implements VoterInterface, CacheableVoterInterface { - public function vote(TokenInterface $token, mixed $subject, array $attributes): int + /** + * @param Vote|null $vote Should be used to explain the vote + */ + public function vote(TokenInterface $token, mixed $subject, array $attributes/* , ?Vote $vote = null */): int { + $vote = 3 < \func_num_args() ? func_get_arg(3) : new Vote(); + $vote ??= new Vote(); // abstain vote by default in case none of the attributes are supported - $vote = self::ACCESS_ABSTAIN; + $vote->result = self::ACCESS_ABSTAIN; foreach ($attributes as $attribute) { try { @@ -43,15 +48,15 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes): } // as soon as at least one attribute is supported, default is to deny access - $vote = self::ACCESS_DENIED; + $vote->result = self::ACCESS_DENIED; - if ($this->voteOnAttribute($attribute, $subject, $token)) { + if ($this->voteOnAttribute($attribute, $subject, $token, $vote)) { // grant access as soon as at least one attribute returns a positive response - return self::ACCESS_GRANTED; + return $vote->result = self::ACCESS_GRANTED; } } - return $vote; + return $vote->result; } /** @@ -90,6 +95,7 @@ abstract protected function supports(string $attribute, mixed $subject): bool; * * @param TAttribute $attribute * @param TSubject $subject + * @param Vote|null $vote Should be used to explain the vote */ - abstract protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool; + abstract protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token/* , ?Vote $vote = null */): bool; } diff --git a/Authorization/Voter/VoterInterface.php b/Authorization/Voter/VoterInterface.php index 5255c88e..0902a94b 100644 --- a/Authorization/Voter/VoterInterface.php +++ b/Authorization/Voter/VoterInterface.php @@ -30,10 +30,11 @@ interface VoterInterface * This method must return one of the following constants: * ACCESS_GRANTED, ACCESS_DENIED, or ACCESS_ABSTAIN. * - * @param mixed $subject The subject to secure - * @param array $attributes An array of attributes associated with the method being invoked + * @param mixed $subject The subject to secure + * @param array $attributes An array of attributes associated with the method being invoked + * @param Vote|null $vote Should be used to explain the vote * * @return self::ACCESS_* */ - public function vote(TokenInterface $token, mixed $subject, array $attributes): int; + public function vote(TokenInterface $token, mixed $subject, array $attributes/* , ?Vote $vote = null */): int; } diff --git a/CHANGELOG.md b/CHANGELOG.md index 290aa8b2..331b204c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ CHANGELOG * Add `OfflineTokenInterface` to mark tokens that do not represent the currently logged-in user * Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`, erase credentials e.g. using `__serialize()` instead + * Add ability for voters to explain their vote 7.2 --- diff --git a/Event/VoteEvent.php b/Event/VoteEvent.php index edc66b36..5842c541 100644 --- a/Event/VoteEvent.php +++ b/Event/VoteEvent.php @@ -28,6 +28,7 @@ public function __construct( private mixed $subject, private array $attributes, private int $vote, + private array $reasons = [], ) { } @@ -50,4 +51,9 @@ public function getVote(): int { return $this->vote; } + + public function getReasons(): array + { + return $this->reasons; + } } diff --git a/Exception/AccessDeniedException.php b/Exception/AccessDeniedException.php index 93c38694..a3e5747e 100644 --- a/Exception/AccessDeniedException.php +++ b/Exception/AccessDeniedException.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Core\Exception; use Symfony\Component\HttpKernel\Attribute\WithHttpStatus; +use Symfony\Component\Security\Core\Authorization\AccessDecision; /** * AccessDeniedException is thrown when the account has not the required role. @@ -23,6 +24,7 @@ class AccessDeniedException extends RuntimeException { private array $attributes = []; private mixed $subject = null; + private ?AccessDecision $accessDecision = null; public function __construct(string $message = 'Access Denied.', ?\Throwable $previous = null, int $code = 403) { @@ -48,4 +50,14 @@ public function setSubject(mixed $subject): void { $this->subject = $subject; } + + public function setAccessDecision(AccessDecision $accessDecision): void + { + $this->accessDecision = $accessDecision; + } + + public function getAccessDecision(): ?AccessDecision + { + return $this->accessDecision; + } } diff --git a/Test/AccessDecisionStrategyTestCase.php b/Test/AccessDecisionStrategyTestCase.php index 792e7779..563a6138 100644 --- a/Test/AccessDecisionStrategyTestCase.php +++ b/Test/AccessDecisionStrategyTestCase.php @@ -16,6 +16,7 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionManager; use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -71,7 +72,7 @@ public function __construct( ) { } - public function vote(TokenInterface $token, $subject, array $attributes): int + public function vote(TokenInterface $token, $subject, array $attributes, ?Vote $vote = null): int { return $this->vote; } diff --git a/Tests/Authorization/TraceableAccessDecisionManagerTest.php b/Tests/Authorization/TraceableAccessDecisionManagerTest.php index 8797d74d..f5313bb5 100644 --- a/Tests/Authorization/TraceableAccessDecisionManagerTest.php +++ b/Tests/Authorization/TraceableAccessDecisionManagerTest.php @@ -61,8 +61,8 @@ public static function provideObjectsAndLogs(): \Generator 'object' => null, 'result' => true, 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['ATTRIBUTE_1'], 'vote' => VoterInterface::ACCESS_GRANTED], - ['voter' => $voter2, 'attributes' => ['ATTRIBUTE_1'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter1, 'attributes' => ['ATTRIBUTE_1'], 'vote' => VoterInterface::ACCESS_GRANTED, 'reasons' => []], + ['voter' => $voter2, 'attributes' => ['ATTRIBUTE_1'], 'vote' => VoterInterface::ACCESS_GRANTED, 'reasons' => []], ], ]], ['ATTRIBUTE_1'], @@ -79,8 +79,8 @@ public static function provideObjectsAndLogs(): \Generator 'object' => true, 'result' => false, 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['ATTRIBUTE_1', 'ATTRIBUTE_2'], 'vote' => VoterInterface::ACCESS_ABSTAIN], - ['voter' => $voter2, 'attributes' => ['ATTRIBUTE_1', 'ATTRIBUTE_2'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter1, 'attributes' => ['ATTRIBUTE_1', 'ATTRIBUTE_2'], 'vote' => VoterInterface::ACCESS_ABSTAIN, 'reasons' => []], + ['voter' => $voter2, 'attributes' => ['ATTRIBUTE_1', 'ATTRIBUTE_2'], 'vote' => VoterInterface::ACCESS_GRANTED, 'reasons' => []], ], ]], ['ATTRIBUTE_1', 'ATTRIBUTE_2'], @@ -97,8 +97,8 @@ public static function provideObjectsAndLogs(): \Generator 'object' => 'jolie string', 'result' => false, 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => [null], 'vote' => VoterInterface::ACCESS_ABSTAIN], - ['voter' => $voter2, 'attributes' => [null], 'vote' => VoterInterface::ACCESS_DENIED], + ['voter' => $voter1, 'attributes' => [null], 'vote' => VoterInterface::ACCESS_ABSTAIN, 'reasons' => []], + ['voter' => $voter2, 'attributes' => [null], 'vote' => VoterInterface::ACCESS_DENIED, 'reasons' => []], ], ]], [null], @@ -139,8 +139,8 @@ public static function provideObjectsAndLogs(): \Generator 'object' => $x = [], 'result' => false, 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['ATTRIBUTE_2'], 'vote' => VoterInterface::ACCESS_ABSTAIN], - ['voter' => $voter2, 'attributes' => ['ATTRIBUTE_2'], 'vote' => VoterInterface::ACCESS_ABSTAIN], + ['voter' => $voter1, 'attributes' => ['ATTRIBUTE_2'], 'vote' => VoterInterface::ACCESS_ABSTAIN, 'reasons' => []], + ['voter' => $voter2, 'attributes' => ['ATTRIBUTE_2'], 'vote' => VoterInterface::ACCESS_ABSTAIN, 'reasons' => []], ], ]], ['ATTRIBUTE_2'], @@ -157,8 +157,8 @@ public static function provideObjectsAndLogs(): \Generator 'object' => new \stdClass(), 'result' => false, 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => [12.13], 'vote' => VoterInterface::ACCESS_DENIED], - ['voter' => $voter2, 'attributes' => [12.13], 'vote' => VoterInterface::ACCESS_DENIED], + ['voter' => $voter1, 'attributes' => [12.13], 'vote' => VoterInterface::ACCESS_DENIED, 'reasons' => []], + ['voter' => $voter2, 'attributes' => [12.13], 'vote' => VoterInterface::ACCESS_DENIED, 'reasons' => []], ], ]], [12.13], @@ -242,7 +242,7 @@ public function testAccessDecisionManagerCalledByVoter() 'attributes' => ['attr1'], 'object' => null, 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['attr1'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter1, 'attributes' => ['attr1'], 'vote' => VoterInterface::ACCESS_GRANTED, 'reasons' => []], ], 'result' => true, ], @@ -250,8 +250,8 @@ public function testAccessDecisionManagerCalledByVoter() 'attributes' => ['attr2'], 'object' => null, 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_ABSTAIN], - ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter1, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_ABSTAIN, 'reasons' => []], + ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_GRANTED, 'reasons' => []], ], 'result' => true, ], @@ -259,9 +259,9 @@ public function testAccessDecisionManagerCalledByVoter() 'attributes' => ['attr2'], 'object' => $obj, 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_ABSTAIN], - ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_DENIED], - ['voter' => $voter3, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter1, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_ABSTAIN, 'reasons' => []], + ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_DENIED, 'reasons' => []], + ['voter' => $voter3, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_GRANTED, 'reasons' => []], ], 'result' => true, ], diff --git a/Tests/Authorization/Voter/VoterTest.php b/Tests/Authorization/Voter/VoterTest.php index 602c61ab..a8f87e09 100644 --- a/Tests/Authorization/Voter/VoterTest.php +++ b/Tests/Authorization/Voter/VoterTest.php @@ -13,6 +13,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\Voter; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; @@ -73,7 +74,7 @@ public function testVoteWithTypeError() class VoterTest_Voter extends Voter { - protected function voteOnAttribute(string $attribute, $object, TokenInterface $token): bool + protected function voteOnAttribute(string $attribute, $object, TokenInterface $token, ?Vote $vote = null): bool { return 'EDIT' === $attribute; } @@ -86,7 +87,7 @@ protected function supports(string $attribute, $object): bool class IntegerVoterTest_Voter extends Voter { - protected function voteOnAttribute($attribute, $object, TokenInterface $token): bool + protected function voteOnAttribute($attribute, $object, TokenInterface $token, ?Vote $vote = null): bool { return 42 === $attribute; } @@ -99,7 +100,7 @@ protected function supports($attribute, $object): bool class TypeErrorVoterTest_Voter extends Voter { - protected function voteOnAttribute($attribute, $object, TokenInterface $token): bool + protected function voteOnAttribute($attribute, $object, TokenInterface $token, ?Vote $vote = null): bool { return false; } diff --git a/Tests/Fixtures/DummyVoter.php b/Tests/Fixtures/DummyVoter.php index 1f923423..16ae8e8a 100644 --- a/Tests/Fixtures/DummyVoter.php +++ b/Tests/Fixtures/DummyVoter.php @@ -12,11 +12,12 @@ namespace Symfony\Component\Security\Core\Tests\Fixtures; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; final class DummyVoter implements VoterInterface { - public function vote(TokenInterface $token, $subject, array $attributes): int + public function vote(TokenInterface $token, $subject, array $attributes, ?Vote $vote = null): int { } } From 4f5954498c035459e0ce0a5a37be83dc8adbcb09 Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Mon, 9 Dec 2024 08:56:13 +0100 Subject: [PATCH 43/52] [Security] Allow using a callable with `#[IsGranted]` --- .../AuthorizationCheckerInterface.php | 2 +- Authorization/Voter/ClosureVoter.php | 78 ++++++++++++++++ CHANGELOG.md | 1 + .../Authorization/Voter/ClosureVoterTest.php | 91 +++++++++++++++++++ 4 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 Authorization/Voter/ClosureVoter.php create mode 100644 Tests/Authorization/Voter/ClosureVoterTest.php diff --git a/Authorization/AuthorizationCheckerInterface.php b/Authorization/AuthorizationCheckerInterface.php index 7c673dfc..848b17ee 100644 --- a/Authorization/AuthorizationCheckerInterface.php +++ b/Authorization/AuthorizationCheckerInterface.php @@ -21,7 +21,7 @@ interface AuthorizationCheckerInterface /** * Checks if the attribute is granted against the current authentication token and optionally supplied subject. * - * @param mixed $attribute A single attribute to vote on (can be of any type, string and instance of Expression are supported by the core) + * @param mixed $attribute A single attribute to vote on (can be of any type; strings, Expression and Closure instances are supported by the core) * @param AccessDecision|null $accessDecision Should be used to explain the decision */ public function isGranted(mixed $attribute, mixed $subject = null/* , ?AccessDecision $accessDecision = null */): bool; diff --git a/Authorization/Voter/ClosureVoter.php b/Authorization/Voter/ClosureVoter.php new file mode 100644 index 00000000..23140c80 --- /dev/null +++ b/Authorization/Voter/ClosureVoter.php @@ -0,0 +1,78 @@ + + * + * 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; + +use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; +use Symfony\Component\Security\Http\Attribute\IsGranted; + +/** + * This voter allows using a closure as the attribute being voted on. + * + * The following named arguments are passed to the closure: + * + * - `token`: The token being used for voting + * - `subject`: The subject of the vote + * - `accessDecisionManager`: The access decision manager + * - `trustResolver`: The trust resolver + * + * @see IsGranted doc for the complete closure signature. + * + * @author Alexandre Daubois + */ +final class ClosureVoter implements CacheableVoterInterface +{ + public function __construct( + private AccessDecisionManagerInterface $accessDecisionManager, + private AuthenticationTrustResolverInterface $trustResolver, + ) { + } + + public function supportsAttribute(string $attribute): bool + { + return false; + } + + public function supportsType(string $subjectType): bool + { + return true; + } + + public function vote(TokenInterface $token, mixed $subject, array $attributes, ?Vote $vote = null): int + { + $vote ??= new Vote(); + $failingClosures = []; + $result = VoterInterface::ACCESS_ABSTAIN; + foreach ($attributes as $attribute) { + if (!$attribute instanceof \Closure) { + continue; + } + + $name = (new \ReflectionFunction($attribute))->name; + $result = VoterInterface::ACCESS_DENIED; + if ($attribute(token: $token, subject: $subject, accessDecisionManager: $this->accessDecisionManager, trustResolver: $this->trustResolver)) { + $vote->reasons[] = \sprintf('Closure %s returned true.', $name); + + return VoterInterface::ACCESS_GRANTED; + } + + $failingClosures[] = $name; + } + + if ($failingClosures) { + $vote->reasons[] = \sprintf('Closure%s %s returned false.', 1 < \count($failingClosures) ? 's' : '', implode(', ', $failingClosures)); + } + + return $result; + } +} diff --git a/CHANGELOG.md b/CHANGELOG.md index 331b204c..12a0c0a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ CHANGELOG * Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`, erase credentials e.g. using `__serialize()` instead * Add ability for voters to explain their vote + * Add support for voting on closures 7.2 --- diff --git a/Tests/Authorization/Voter/ClosureVoterTest.php b/Tests/Authorization/Voter/ClosureVoterTest.php new file mode 100644 index 00000000..a919916a --- /dev/null +++ b/Tests/Authorization/Voter/ClosureVoterTest.php @@ -0,0 +1,91 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Tests\Authorization\Voter; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; +use Symfony\Component\Security\Core\Authorization\Voter\ClosureVoter; +use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; +use Symfony\Component\Security\Core\User\UserInterface; + +class ClosureVoterTest extends TestCase +{ + private ClosureVoter $voter; + + protected function setUp(): void + { + $this->voter = new ClosureVoter( + $this->createMock(AccessDecisionManagerInterface::class), + $this->createMock(AuthenticationTrustResolverInterface::class), + ); + } + + public function testEmptyAttributeAbstains() + { + $this->assertSame(VoterInterface::ACCESS_ABSTAIN, $this->voter->vote( + $this->createMock(TokenInterface::class), + null, + []) + ); + } + + public function testClosureReturningFalseDeniesAccess() + { + $token = $this->createMock(TokenInterface::class); + $token->method('getRoleNames')->willReturn([]); + $token->method('getUser')->willReturn($this->createMock(UserInterface::class)); + + $this->assertSame(VoterInterface::ACCESS_DENIED, $this->voter->vote( + $token, + null, + [fn (...$vars) => false] + )); + } + + public function testClosureReturningTrueGrantsAccess() + { + $token = $this->createMock(TokenInterface::class); + $token->method('getRoleNames')->willReturn([]); + $token->method('getUser')->willReturn($this->createMock(UserInterface::class)); + + $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->voter->vote( + $token, + null, + [fn (...$vars) => true] + )); + } + + public function testArgumentsContent() + { + $token = $this->createMock(TokenInterface::class); + $token->method('getRoleNames')->willReturn(['MY_ROLE', 'ANOTHER_ROLE']); + $token->method('getUser')->willReturn($this->createMock(UserInterface::class)); + + $outerSubject = new \stdClass(); + + $this->voter->vote( + $token, + $outerSubject, + [function (...$vars) use ($outerSubject) { + $this->assertInstanceOf(TokenInterface::class, $vars['token']); + $this->assertSame($outerSubject, $vars['subject']); + + $this->assertInstanceOf(AccessDecisionManagerInterface::class, $vars['accessDecisionManager']); + $this->assertInstanceOf(AuthenticationTrustResolverInterface::class, $vars['trustResolver']); + + return true; + }] + ); + } +} From 97778ec867f16b74baaec312a9ed0e70b8d5eaa6 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 17 Feb 2025 16:31:27 +0100 Subject: [PATCH 44/52] [Security] Improve DX of recent additions --- .../Token/UserAuthorizationCheckerToken.php | 31 -------- Authorization/AuthorizationChecker.php | 21 +++++- Authorization/UserAuthorizationChecker.php | 31 -------- Authorization/Voter/ClosureVoter.php | 17 ++--- CHANGELOG.md | 3 +- .../UserAuthorizationCheckerTokenTest.php | 26 ------- .../AuthorizationCheckerTest.php | 39 +++++++++++ .../UserAuthorizationCheckerTest.php | 70 ------------------- .../Voter/AuthenticatedVoterTest.php | 4 +- .../Authorization/Voter/ClosureVoterTest.php | 22 +++--- 10 files changed, 76 insertions(+), 188 deletions(-) delete mode 100644 Authentication/Token/UserAuthorizationCheckerToken.php delete mode 100644 Authorization/UserAuthorizationChecker.php delete mode 100644 Tests/Authentication/Token/UserAuthorizationCheckerTokenTest.php delete mode 100644 Tests/Authorization/UserAuthorizationCheckerTest.php diff --git a/Authentication/Token/UserAuthorizationCheckerToken.php b/Authentication/Token/UserAuthorizationCheckerToken.php deleted file mode 100644 index 2e84ce7a..00000000 --- a/Authentication/Token/UserAuthorizationCheckerToken.php +++ /dev/null @@ -1,31 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Security\Core\Authentication\Token; - -use Symfony\Component\Security\Core\User\UserInterface; - -/** - * UserAuthorizationCheckerToken implements a token used for checking authorization. - * - * @author Nate Wiebe - * - * @internal - */ -final class UserAuthorizationCheckerToken extends AbstractToken implements OfflineTokenInterface -{ - public function __construct(UserInterface $user) - { - parent::__construct($user->getRoles()); - - $this->setUser($user); - } -} diff --git a/Authorization/AuthorizationChecker.php b/Authorization/AuthorizationChecker.php index 3960f2be..3689bf5e 100644 --- a/Authorization/AuthorizationChecker.php +++ b/Authorization/AuthorizationChecker.php @@ -11,8 +11,11 @@ namespace Symfony\Component\Security\Core\Authorization; +use Symfony\Component\Security\Core\Authentication\Token\AbstractToken; use Symfony\Component\Security\Core\Authentication\Token\NullToken; +use Symfony\Component\Security\Core\Authentication\Token\OfflineTokenInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; +use Symfony\Component\Security\Core\User\UserInterface; /** * AuthorizationChecker is the main authorization point of the Security component. @@ -22,8 +25,9 @@ * @author Fabien Potencier * @author Johannes M. Schmitt */ -class AuthorizationChecker implements AuthorizationCheckerInterface +class AuthorizationChecker implements AuthorizationCheckerInterface, UserAuthorizationCheckerInterface { + private array $tokenStack = []; private array $accessDecisionStack = []; public function __construct( @@ -34,7 +38,7 @@ public function __construct( final public function isGranted(mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool { - $token = $this->tokenStorage->getToken(); + $token = end($this->tokenStack) ?: $this->tokenStorage->getToken(); if (!$token || !$token->getUser()) { $token = new NullToken(); @@ -48,4 +52,17 @@ final public function isGranted(mixed $attribute, mixed $subject = null, ?Access array_pop($this->accessDecisionStack); } } + + final public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool + { + $token = new class($user->getRoles()) extends AbstractToken implements OfflineTokenInterface {}; + $token->setUser($user); + $this->tokenStack[] = $token; + + try { + return $this->isGranted($attribute, $subject, $accessDecision); + } finally { + array_pop($this->tokenStack); + } + } } diff --git a/Authorization/UserAuthorizationChecker.php b/Authorization/UserAuthorizationChecker.php deleted file mode 100644 index f515e5cb..00000000 --- a/Authorization/UserAuthorizationChecker.php +++ /dev/null @@ -1,31 +0,0 @@ - - * - * 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; - -use Symfony\Component\Security\Core\Authentication\Token\UserAuthorizationCheckerToken; -use Symfony\Component\Security\Core\User\UserInterface; - -/** - * @author Nate Wiebe - */ -final class UserAuthorizationChecker implements UserAuthorizationCheckerInterface -{ - public function __construct( - private readonly AccessDecisionManagerInterface $accessDecisionManager, - ) { - } - - public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool - { - return $this->accessDecisionManager->decide(new UserAuthorizationCheckerToken($user), [$attribute], $subject, $accessDecision); - } -} diff --git a/Authorization/Voter/ClosureVoter.php b/Authorization/Voter/ClosureVoter.php index 23140c80..03a9f757 100644 --- a/Authorization/Voter/ClosureVoter.php +++ b/Authorization/Voter/ClosureVoter.php @@ -11,21 +11,14 @@ namespace Symfony\Component\Security\Core\Authorization\Voter; -use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; -use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; +use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; use Symfony\Component\Security\Http\Attribute\IsGranted; +use Symfony\Component\Security\Http\Attribute\IsGrantedContext; /** * This voter allows using a closure as the attribute being voted on. * - * The following named arguments are passed to the closure: - * - * - `token`: The token being used for voting - * - `subject`: The subject of the vote - * - `accessDecisionManager`: The access decision manager - * - `trustResolver`: The trust resolver - * * @see IsGranted doc for the complete closure signature. * * @author Alexandre Daubois @@ -33,8 +26,7 @@ final class ClosureVoter implements CacheableVoterInterface { public function __construct( - private AccessDecisionManagerInterface $accessDecisionManager, - private AuthenticationTrustResolverInterface $trustResolver, + private AuthorizationCheckerInterface $authorizationChecker, ) { } @@ -51,6 +43,7 @@ public function supportsType(string $subjectType): bool public function vote(TokenInterface $token, mixed $subject, array $attributes, ?Vote $vote = null): int { $vote ??= new Vote(); + $context = new IsGrantedContext($token, $token->getUser(), $this->authorizationChecker); $failingClosures = []; $result = VoterInterface::ACCESS_ABSTAIN; foreach ($attributes as $attribute) { @@ -60,7 +53,7 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes, ? $name = (new \ReflectionFunction($attribute))->name; $result = VoterInterface::ACCESS_DENIED; - if ($attribute(token: $token, subject: $subject, accessDecisionManager: $this->accessDecisionManager, trustResolver: $this->trustResolver)) { + if ($attribute($context, $subject)) { $vote->reasons[] = \sprintf('Closure %s returned true.', $name); return VoterInterface::ACCESS_GRANTED; diff --git a/CHANGELOG.md b/CHANGELOG.md index 12a0c0a6..d3b3b652 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,7 @@ CHANGELOG 7.3 --- - * Add `UserAuthorizationChecker::isGrantedForUser()` to test user authorization without relying on the session. - For example, users not currently logged in, or while processing a message from a message queue. + * Add `UserAuthorizationCheckerInterface` to test user authorization without relying on the session * Add `OfflineTokenInterface` to mark tokens that do not represent the currently logged-in user * Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`, erase credentials e.g. using `__serialize()` instead diff --git a/Tests/Authentication/Token/UserAuthorizationCheckerTokenTest.php b/Tests/Authentication/Token/UserAuthorizationCheckerTokenTest.php deleted file mode 100644 index 2e7e11bd..00000000 --- a/Tests/Authentication/Token/UserAuthorizationCheckerTokenTest.php +++ /dev/null @@ -1,26 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Security\Core\Tests\Authentication\Token; - -use PHPUnit\Framework\TestCase; -use Symfony\Component\Security\Core\Authentication\Token\UserAuthorizationCheckerToken; -use Symfony\Component\Security\Core\User\InMemoryUser; - -class UserAuthorizationCheckerTokenTest extends TestCase -{ - public function testConstructor() - { - $token = new UserAuthorizationCheckerToken($user = new InMemoryUser('foo', 'bar', ['ROLE_FOO'])); - $this->assertSame(['ROLE_FOO'], $token->getRoleNames()); - $this->assertSame($user, $token->getUser()); - } -} diff --git a/Tests/Authorization/AuthorizationCheckerTest.php b/Tests/Authorization/AuthorizationCheckerTest.php index 36b048c8..00f0f50e 100644 --- a/Tests/Authorization/AuthorizationCheckerTest.php +++ b/Tests/Authorization/AuthorizationCheckerTest.php @@ -14,6 +14,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\Token\NullToken; +use Symfony\Component\Security\Core\Authentication\Token\OfflineTokenInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; @@ -77,4 +78,42 @@ public function testIsGrantedWithObjectAttribute() $this->tokenStorage->setToken($token); $this->assertTrue($this->authorizationChecker->isGranted($attribute)); } + + /** + * @dataProvider isGrantedForUserProvider + */ + public function testIsGrantedForUser(bool $decide, array $roles) + { + $user = new InMemoryUser('username', 'password', $roles); + + $this->accessDecisionManager + ->expects($this->once()) + ->method('decide') + ->with($this->callback(static fn (OfflineTokenInterface $token) => $token->getUser() === $user), ['ROLE_FOO']) + ->willReturn($decide); + + $this->assertSame($decide, $this->authorizationChecker->isGrantedForUser($user, 'ROLE_FOO')); + } + + public static function isGrantedForUserProvider(): array + { + return [ + [false, ['ROLE_USER']], + [true, ['ROLE_USER', 'ROLE_FOO']], + ]; + } + + public function testIsGrantedForUserWithObjectAttribute() + { + $attribute = new \stdClass(); + + $user = new InMemoryUser('username', 'password', ['ROLE_USER']); + + $this->accessDecisionManager + ->expects($this->once()) + ->method('decide') + ->with($this->isInstanceOf(OfflineTokenInterface::class), [$attribute]) + ->willReturn(true); + $this->assertTrue($this->authorizationChecker->isGrantedForUser($user, $attribute)); + } } diff --git a/Tests/Authorization/UserAuthorizationCheckerTest.php b/Tests/Authorization/UserAuthorizationCheckerTest.php deleted file mode 100644 index e9b6bb74..00000000 --- a/Tests/Authorization/UserAuthorizationCheckerTest.php +++ /dev/null @@ -1,70 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Security\Core\Tests\Authorization; - -use PHPUnit\Framework\MockObject\MockObject; -use PHPUnit\Framework\TestCase; -use Symfony\Component\Security\Core\Authentication\Token\UserAuthorizationCheckerToken; -use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; -use Symfony\Component\Security\Core\Authorization\UserAuthorizationChecker; -use Symfony\Component\Security\Core\User\InMemoryUser; - -class UserAuthorizationCheckerTest extends TestCase -{ - private AccessDecisionManagerInterface&MockObject $accessDecisionManager; - private UserAuthorizationChecker $authorizationChecker; - - protected function setUp(): void - { - $this->accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); - - $this->authorizationChecker = new UserAuthorizationChecker($this->accessDecisionManager); - } - - /** - * @dataProvider isGrantedProvider - */ - public function testIsGranted(bool $decide, array $roles) - { - $user = new InMemoryUser('username', 'password', $roles); - - $this->accessDecisionManager - ->expects($this->once()) - ->method('decide') - ->with($this->callback(fn (UserAuthorizationCheckerToken $token): bool => $user === $token->getUser()), $this->identicalTo(['ROLE_FOO'])) - ->willReturn($decide); - - $this->assertSame($decide, $this->authorizationChecker->isGrantedForUser($user, 'ROLE_FOO')); - } - - public static function isGrantedProvider(): array - { - return [ - [false, ['ROLE_USER']], - [true, ['ROLE_USER', 'ROLE_FOO']], - ]; - } - - public function testIsGrantedWithObjectAttribute() - { - $attribute = new \stdClass(); - - $token = new UserAuthorizationCheckerToken(new InMemoryUser('username', 'password', ['ROLE_USER'])); - - $this->accessDecisionManager - ->expects($this->once()) - ->method('decide') - ->with($this->isInstanceOf($token::class), $this->identicalTo([$attribute])) - ->willReturn(true); - $this->assertTrue($this->authorizationChecker->isGrantedForUser($token->getUser(), $attribute)); - } -} diff --git a/Tests/Authorization/Voter/AuthenticatedVoterTest.php b/Tests/Authorization/Voter/AuthenticatedVoterTest.php index 89f6c350..b5e0bf42 100644 --- a/Tests/Authorization/Voter/AuthenticatedVoterTest.php +++ b/Tests/Authorization/Voter/AuthenticatedVoterTest.php @@ -15,9 +15,9 @@ use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver; use Symfony\Component\Security\Core\Authentication\Token\AbstractToken; use Symfony\Component\Security\Core\Authentication\Token\NullToken; +use Symfony\Component\Security\Core\Authentication\Token\OfflineTokenInterface; use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken; use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; -use Symfony\Component\Security\Core\Authentication\Token\UserAuthorizationCheckerToken; use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Exception\InvalidArgumentException; @@ -148,7 +148,7 @@ public function getCredentials() } if ('offline' === $authenticated) { - return new UserAuthorizationCheckerToken($user); + return new class($user->getRoles()) extends AbstractToken implements OfflineTokenInterface {}; } return new NullToken(); diff --git a/Tests/Authorization/Voter/ClosureVoterTest.php b/Tests/Authorization/Voter/ClosureVoterTest.php index a919916a..7a22f2d4 100644 --- a/Tests/Authorization/Voter/ClosureVoterTest.php +++ b/Tests/Authorization/Voter/ClosureVoterTest.php @@ -12,13 +12,16 @@ namespace Symfony\Component\Security\Core\Tests\Authorization\Voter; use PHPUnit\Framework\TestCase; -use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; -use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; +use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; use Symfony\Component\Security\Core\Authorization\Voter\ClosureVoter; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\User\UserInterface; +use Symfony\Component\Security\Http\Attribute\IsGrantedContext; +/** + * @requires function Symfony\Component\Security\Http\Attribute\IsGrantedContext::isGranted + */ class ClosureVoterTest extends TestCase { private ClosureVoter $voter; @@ -26,8 +29,7 @@ class ClosureVoterTest extends TestCase protected function setUp(): void { $this->voter = new ClosureVoter( - $this->createMock(AccessDecisionManagerInterface::class), - $this->createMock(AuthenticationTrustResolverInterface::class), + $this->createMock(AuthorizationCheckerInterface::class), ); } @@ -49,7 +51,7 @@ public function testClosureReturningFalseDeniesAccess() $this->assertSame(VoterInterface::ACCESS_DENIED, $this->voter->vote( $token, null, - [fn (...$vars) => false] + [fn () => false] )); } @@ -62,7 +64,7 @@ public function testClosureReturningTrueGrantsAccess() $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->voter->vote( $token, null, - [fn (...$vars) => true] + [fn () => true] )); } @@ -77,12 +79,8 @@ public function testArgumentsContent() $this->voter->vote( $token, $outerSubject, - [function (...$vars) use ($outerSubject) { - $this->assertInstanceOf(TokenInterface::class, $vars['token']); - $this->assertSame($outerSubject, $vars['subject']); - - $this->assertInstanceOf(AccessDecisionManagerInterface::class, $vars['accessDecisionManager']); - $this->assertInstanceOf(AuthenticationTrustResolverInterface::class, $vars['trustResolver']); + [function (IsGrantedContext $context, \stdClass $subject) use ($outerSubject) { + $this->assertSame($outerSubject, $subject); return true; }] From 41843c826b843ce843b09c37d6c837cc4d293a20 Mon Sep 17 00:00:00 2001 From: Florent Morselli Date: Thu, 13 Feb 2025 09:05:26 +0100 Subject: [PATCH 45/52] [Security] OAuth2 Introspection Endpoint (RFC7662) In addition to the excellent work of @vincentchalamon #48272, this PR allows getting the data from the OAuth2 Introspection Endpoint. This endpoint is defined in the [RFC7662](https://datatracker.ietf.org/doc/html/rfc7662). It returns the following information that is used to retrieve the user: * If the access token is active * A set of claims that are similar to the OIDC one, including the `sub` or the `username`. --- CHANGELOG.md | 1 + Tests/User/OAuth2UserTest.php | 51 +++++++++++++++++++++++++ User/OAuth2User.php | 70 +++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+) create mode 100644 Tests/User/OAuth2UserTest.php create mode 100644 User/OAuth2User.php diff --git a/CHANGELOG.md b/CHANGELOG.md index d3b3b652..12806416 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ CHANGELOG erase credentials e.g. using `__serialize()` instead * Add ability for voters to explain their vote * Add support for voting on closures + * Add `OAuth2User` with OAuth2 Access Token Introspection support for `OAuth2TokenHandler` 7.2 --- diff --git a/Tests/User/OAuth2UserTest.php b/Tests/User/OAuth2UserTest.php new file mode 100644 index 00000000..a53ed1b5 --- /dev/null +++ b/Tests/User/OAuth2UserTest.php @@ -0,0 +1,51 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Tests\User; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\Security\Core\User\OAuth2User; + +class OAuth2UserTest extends TestCase +{ + public function testCannotCreateUserWithoutSubProperty() + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('The claim "sub" or "username" must be provided.'); + + new OAuth2User(); + } + + public function testCreateFullUserWithAdditionalClaimsUsingPositionalParameters() + { + $this->assertEquals(new OAuth2User( + scope: 'read write dolphin', + username: 'jdoe', + exp: 1419356238, + iat: 1419350238, + sub: 'Z5O3upPC88QrAjx00dis', + aud: 'https://protected.example.net/resource', + iss: 'https://server.example.com/', + client_id: 'l238j323ds-23ij4', + extension_field: 'twenty-seven' + ), new OAuth2User(...[ + 'client_id' => 'l238j323ds-23ij4', + 'username' => 'jdoe', + 'scope' => 'read write dolphin', + 'sub' => 'Z5O3upPC88QrAjx00dis', + 'aud' => 'https://protected.example.net/resource', + 'iss' => 'https://server.example.com/', + 'exp' => 1419356238, + 'iat' => 1419350238, + 'extension_field' => 'twenty-seven', + ])); + } +} diff --git a/User/OAuth2User.php b/User/OAuth2User.php new file mode 100644 index 00000000..42c0550a --- /dev/null +++ b/User/OAuth2User.php @@ -0,0 +1,70 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\User; + +/** + * UserInterface implementation used by the access-token security workflow with an OIDC server. + */ +class OAuth2User implements UserInterface +{ + public readonly array $additionalClaims; + + public function __construct( + private array $roles = ['ROLE_USER'], + // Standard Claims (https://datatracker.ietf.org/doc/html/rfc7662#section-2.2) + public readonly ?string $scope = null, + public readonly ?string $clientId = null, + public readonly ?string $username = null, + public readonly ?string $tokenType = null, + public readonly ?int $exp = null, + public readonly ?int $iat = null, + public readonly ?int $nbf = null, + public readonly ?string $sub = null, + public readonly ?string $aud = null, + public readonly ?string $iss = null, + public readonly ?string $jti = null, + + // Additional Claims (" + // Specific implementations MAY extend this structure with + // their own service-specific response names as top-level members + // of this JSON object. + // ") + ...$additionalClaims, + ) { + if ((null === $sub || '' === $sub) && (null === $username || '' === $username)) { + throw new \InvalidArgumentException('The claim "sub" or "username" must be provided.'); + } + + $this->additionalClaims = $additionalClaims['additionalClaims'] ?? $additionalClaims; + } + + /** + * OIDC or OAuth specs don't have any "role" notion. + * + * If you want to implement "roles" from your OIDC server, + * send a "roles" constructor argument to this object + * (e.g.: using a custom UserProvider). + */ + public function getRoles(): array + { + return $this->roles; + } + + public function getUserIdentifier(): string + { + return (string) ($this->sub ?? $this->username); + } + + public function eraseCredentials(): void + { + } +} From 7929409caf9dc6071df627e8c19a57c222625fe5 Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Thu, 20 Mar 2025 12:22:14 +0100 Subject: [PATCH 46/52] Improve documentation of PasswordUpgraderInterface --- User/PasswordUpgraderInterface.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/User/PasswordUpgraderInterface.php b/User/PasswordUpgraderInterface.php index fd21f14a..2f200ccb 100644 --- a/User/PasswordUpgraderInterface.php +++ b/User/PasswordUpgraderInterface.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Security\Core\User; +use Symfony\Component\Security\Core\Exception\UnsupportedUserException; + /** * @author Nicolas Grekas */ @@ -22,6 +24,8 @@ interface PasswordUpgraderInterface * This method should persist the new password in the user storage and update the $user object accordingly. * Because you don't want your users not being able to log in, this method should be opportunistic: * it's fine if it does nothing or if it fails without throwing any exception. + * + * @throws UnsupportedUserException if the implementation does not support that user */ public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void; } From 68ebebf88f688138c571f4853a2f68be49e8c159 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Fri, 4 Apr 2025 15:02:15 +0200 Subject: [PATCH 47/52] replace expectDeprecation() with expectUserDeprecationMessage() --- Tests/Authentication/Token/AbstractTokenTest.php | 6 +++--- Tests/User/InMemoryUserTest.php | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Tests/Authentication/Token/AbstractTokenTest.php b/Tests/Authentication/Token/AbstractTokenTest.php index ef3d380c..3972b1cd 100644 --- a/Tests/Authentication/Token/AbstractTokenTest.php +++ b/Tests/Authentication/Token/AbstractTokenTest.php @@ -12,7 +12,7 @@ namespace Symfony\Component\Security\Core\Tests\Authentication\Token; use PHPUnit\Framework\TestCase; -use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; +use Symfony\Bridge\PhpUnit\ExpectUserDeprecationMessageTrait; use Symfony\Component\Security\Core\Authentication\Token\AbstractToken; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\User\InMemoryUser; @@ -20,7 +20,7 @@ class AbstractTokenTest extends TestCase { - use ExpectDeprecationTrait; + use ExpectUserDeprecationMessageTrait; /** * @dataProvider provideUsers @@ -48,7 +48,7 @@ public function testEraseCredentials() $user->expects($this->once())->method('eraseCredentials'); $token->setUser($user); - $this->expectDeprecation(\sprintf('Since symfony/security-core 7.3: The "%s::eraseCredentials()" method is deprecated and will be removed in 8.0, erase credentials using the "__serialize()" method instead.', TokenInterface::class)); + $this->expectUserDeprecationMessage(\sprintf('Since symfony/security-core 7.3: The "%s::eraseCredentials()" method is deprecated and will be removed in 8.0, erase credentials using the "__serialize()" method instead.', TokenInterface::class)); $token->eraseCredentials(); } diff --git a/Tests/User/InMemoryUserTest.php b/Tests/User/InMemoryUserTest.php index 501bf742..f06e98c3 100644 --- a/Tests/User/InMemoryUserTest.php +++ b/Tests/User/InMemoryUserTest.php @@ -12,13 +12,13 @@ namespace Symfony\Component\Security\Core\Tests\User; use PHPUnit\Framework\TestCase; -use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; +use Symfony\Bridge\PhpUnit\ExpectUserDeprecationMessageTrait; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Core\User\UserInterface; class InMemoryUserTest extends TestCase { - use ExpectDeprecationTrait; + use ExpectUserDeprecationMessageTrait; public function testConstructorException() { @@ -62,7 +62,7 @@ public function testIsEnabled() public function testEraseCredentials() { $user = new InMemoryUser('fabien', 'superpass'); - $this->expectDeprecation(\sprintf('%sMethod %s::eraseCredentials() is deprecated since symfony/security-core 7.3', \PHP_VERSION_ID >= 80400 ? 'Unsilenced deprecation: ' : '', InMemoryUser::class)); + $this->expectUserDeprecationMessage(\sprintf('%sMethod %s::eraseCredentials() is deprecated since symfony/security-core 7.3', \PHP_VERSION_ID >= 80400 ? 'Unsilenced deprecation: ' : '', InMemoryUser::class)); $user->eraseCredentials(); $this->assertEquals('superpass', $user->getPassword()); } From 157d3c9fe8c56abe6d6b00a8453be6ce7c430e25 Mon Sep 17 00:00:00 2001 From: Santiago San Martin Date: Sun, 11 May 2025 19:35:25 -0300 Subject: [PATCH 48/52] fix(security): allow multiple Security attributes when applicable --- .../TraceableAccessDecisionManager.php | 2 +- .../TraceableAccessDecisionManagerTest.php | 46 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/Authorization/TraceableAccessDecisionManager.php b/Authorization/TraceableAccessDecisionManager.php index a03e2d0c..0ef062f6 100644 --- a/Authorization/TraceableAccessDecisionManager.php +++ b/Authorization/TraceableAccessDecisionManager.php @@ -54,7 +54,7 @@ public function decide(TokenInterface $token, array $attributes, mixed $object = $this->accessDecisionStack[] = $accessDecision; try { - return $accessDecision->isGranted = $this->manager->decide($token, $attributes, $object, $accessDecision); + return $accessDecision->isGranted = $this->manager->decide($token, $attributes, $object, $accessDecision, $allowMultipleAttributes); } finally { $this->strategy = $accessDecision->strategy; $currentLog = array_pop($this->currentLog); diff --git a/Tests/Authorization/TraceableAccessDecisionManagerTest.php b/Tests/Authorization/TraceableAccessDecisionManagerTest.php index f5313bb5..4bd9a01a 100644 --- a/Tests/Authorization/TraceableAccessDecisionManagerTest.php +++ b/Tests/Authorization/TraceableAccessDecisionManagerTest.php @@ -11,12 +11,14 @@ namespace Symfony\Component\Security\Core\Tests\Authorization; +use PHPUnit\Framework\MockObject\MockObject; 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\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; +use Symfony\Component\Security\Core\Exception\InvalidArgumentException; use Symfony\Component\Security\Core\Tests\Fixtures\DummyVoter; class TraceableAccessDecisionManagerTest extends TestCase @@ -276,4 +278,48 @@ public function testCustomAccessDecisionManagerReturnsEmptyStrategy() $this->assertEquals('-', $adm->getStrategy()); } + + public function testThrowsExceptionWhenMultipleAttributesNotAllowed() + { + $accessDecisionManager = new AccessDecisionManager(); + $traceableAccessDecisionManager = new TraceableAccessDecisionManager($accessDecisionManager); + /** @var TokenInterface&MockObject $tokenMock */ + $tokenMock = $this->createMock(TokenInterface::class); + + $this->expectException(InvalidArgumentException::class); + $traceableAccessDecisionManager->decide($tokenMock, ['attr1', 'attr2']); + } + + /** + * @dataProvider allowMultipleAttributesProvider + */ + public function testAllowMultipleAttributes(array $attributes, bool $allowMultipleAttributes) + { + $accessDecisionManager = new AccessDecisionManager(); + $traceableAccessDecisionManager = new TraceableAccessDecisionManager($accessDecisionManager); + /** @var TokenInterface&MockObject $tokenMock */ + $tokenMock = $this->createMock(TokenInterface::class); + + $isGranted = $traceableAccessDecisionManager->decide($tokenMock, $attributes, null, null, $allowMultipleAttributes); + + $this->assertFalse($isGranted); + } + + public function allowMultipleAttributesProvider(): \Generator + { + yield [ + ['attr1'], + false, + ]; + + yield [ + ['attr1'], + true, + ]; + + yield [ + ['attr1', 'attr2', 'attr3'], + true, + ]; + } } From c6fcc7118f478f40a3f1b74e1ad720d080d47fbd Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Tue, 13 May 2025 09:45:01 +0200 Subject: [PATCH 49/52] [Security] Make data provider static --- Tests/Authorization/TraceableAccessDecisionManagerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Authorization/TraceableAccessDecisionManagerTest.php b/Tests/Authorization/TraceableAccessDecisionManagerTest.php index 4bd9a01a..496d970c 100644 --- a/Tests/Authorization/TraceableAccessDecisionManagerTest.php +++ b/Tests/Authorization/TraceableAccessDecisionManagerTest.php @@ -305,7 +305,7 @@ public function testAllowMultipleAttributes(array $attributes, bool $allowMultip $this->assertFalse($isGranted); } - public function allowMultipleAttributesProvider(): \Generator + public static function allowMultipleAttributesProvider(): \Generator { yield [ ['attr1'], From ea9789fa09c6cbb16b345bcf3a718b8ada8affdb Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Mon, 26 May 2025 11:36:54 +0200 Subject: [PATCH 50/52] do not construct Vote instances inside vote() The so constructed objects will never be seen from the outside. Thus, adding reasons to them doesn't have an effect. --- Authorization/Voter/AuthenticatedVoter.php | 17 ++++++++-------- Authorization/Voter/ClosureVoter.php | 5 ++--- Authorization/Voter/ExpressionVoter.php | 7 +++---- Authorization/Voter/RoleVoter.php | 7 +++---- Authorization/Voter/TraceableVoter.php | 4 ++-- Authorization/Voter/Voter.php | 23 ++++++++++++++++------ Tests/Authorization/Voter/VoterTest.php | 20 +++++++++++++++++-- 7 files changed, 53 insertions(+), 30 deletions(-) diff --git a/Authorization/Voter/AuthenticatedVoter.php b/Authorization/Voter/AuthenticatedVoter.php index 1403aaaa..3ab6b92c 100644 --- a/Authorization/Voter/AuthenticatedVoter.php +++ b/Authorization/Voter/AuthenticatedVoter.php @@ -45,11 +45,10 @@ public function __construct( */ public function vote(TokenInterface $token, mixed $subject, array $attributes/* , ?Vote $vote = null */): int { - $vote = 3 < \func_num_args() ? func_get_arg(3) : new Vote(); - $vote ??= new Vote(); + $vote = 3 < \func_num_args() ? func_get_arg(3) : null; if ($attributes === [self::PUBLIC_ACCESS]) { - $vote->reasons[] = 'Access is public.'; + $vote?->addReason('Access is public.'); return VoterInterface::ACCESS_GRANTED; } @@ -73,7 +72,7 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes/* if ((self::IS_AUTHENTICATED_FULLY === $attribute || self::IS_AUTHENTICATED_REMEMBERED === $attribute) && $this->authenticationTrustResolver->isFullFledged($token) ) { - $vote->reasons[] = 'The user is fully authenticated.'; + $vote?->addReason('The user is fully authenticated.'); return VoterInterface::ACCESS_GRANTED; } @@ -81,32 +80,32 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes/* if (self::IS_AUTHENTICATED_REMEMBERED === $attribute && $this->authenticationTrustResolver->isRememberMe($token) ) { - $vote->reasons[] = 'The user is remembered.'; + $vote?->addReason('The user is remembered.'); return VoterInterface::ACCESS_GRANTED; } if (self::IS_AUTHENTICATED === $attribute && $this->authenticationTrustResolver->isAuthenticated($token)) { - $vote->reasons[] = 'The user is authenticated.'; + $vote?->addReason('The user is authenticated.'); return VoterInterface::ACCESS_GRANTED; } if (self::IS_REMEMBERED === $attribute && $this->authenticationTrustResolver->isRememberMe($token)) { - $vote->reasons[] = 'The user is remembered.'; + $vote?->addReason('The user is remembered.'); return VoterInterface::ACCESS_GRANTED; } if (self::IS_IMPERSONATOR === $attribute && $token instanceof SwitchUserToken) { - $vote->reasons[] = 'The user is impersonating another user.'; + $vote?->addReason('The user is impersonating another user.'); return VoterInterface::ACCESS_GRANTED; } } if (VoterInterface::ACCESS_DENIED === $result) { - $vote->reasons[] = 'The user is not appropriately authenticated.'; + $vote?->addReason('The user is not appropriately authenticated.'); } return $result; diff --git a/Authorization/Voter/ClosureVoter.php b/Authorization/Voter/ClosureVoter.php index 03a9f757..4fb5502f 100644 --- a/Authorization/Voter/ClosureVoter.php +++ b/Authorization/Voter/ClosureVoter.php @@ -42,7 +42,6 @@ public function supportsType(string $subjectType): bool public function vote(TokenInterface $token, mixed $subject, array $attributes, ?Vote $vote = null): int { - $vote ??= new Vote(); $context = new IsGrantedContext($token, $token->getUser(), $this->authorizationChecker); $failingClosures = []; $result = VoterInterface::ACCESS_ABSTAIN; @@ -54,7 +53,7 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes, ? $name = (new \ReflectionFunction($attribute))->name; $result = VoterInterface::ACCESS_DENIED; if ($attribute($context, $subject)) { - $vote->reasons[] = \sprintf('Closure %s returned true.', $name); + $vote?->addReason(\sprintf('Closure %s returned true.', $name)); return VoterInterface::ACCESS_GRANTED; } @@ -63,7 +62,7 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes, ? } if ($failingClosures) { - $vote->reasons[] = \sprintf('Closure%s %s returned false.', 1 < \count($failingClosures) ? 's' : '', implode(', ', $failingClosures)); + $vote?->addReason(\sprintf('Closure%s %s returned false.', 1 < \count($failingClosures) ? 's' : '', implode(', ', $failingClosures))); } return $result; diff --git a/Authorization/Voter/ExpressionVoter.php b/Authorization/Voter/ExpressionVoter.php index 35d727a8..719aae7d 100644 --- a/Authorization/Voter/ExpressionVoter.php +++ b/Authorization/Voter/ExpressionVoter.php @@ -49,8 +49,7 @@ public function supportsType(string $subjectType): bool */ public function vote(TokenInterface $token, mixed $subject, array $attributes/* , ?Vote $vote = null */): int { - $vote = 3 < \func_num_args() ? func_get_arg(3) : new Vote(); - $vote ??= new Vote(); + $vote = 3 < \func_num_args() ? func_get_arg(3) : null; $result = VoterInterface::ACCESS_ABSTAIN; $variables = null; $failingExpressions = []; @@ -64,7 +63,7 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes/* $result = VoterInterface::ACCESS_DENIED; if ($this->expressionLanguage->evaluate($attribute, $variables)) { - $vote->reasons[] = \sprintf('Expression (%s) is true.', $attribute); + $vote?->addReason(\sprintf('Expression (%s) is true.', $attribute)); return VoterInterface::ACCESS_GRANTED; } @@ -73,7 +72,7 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes/* } if ($failingExpressions) { - $vote->reasons[] = \sprintf('Expression (%s) is false.', implode(') || (', $failingExpressions)); + $vote?->addReason(\sprintf('Expression (%s) is false.', implode(') || (', $failingExpressions))); } return $result; diff --git a/Authorization/Voter/RoleVoter.php b/Authorization/Voter/RoleVoter.php index 46c08d15..2225e8d4 100644 --- a/Authorization/Voter/RoleVoter.php +++ b/Authorization/Voter/RoleVoter.php @@ -30,8 +30,7 @@ public function __construct( */ public function vote(TokenInterface $token, mixed $subject, array $attributes/* , ?Vote $vote = null */): int { - $vote = 3 < \func_num_args() ? func_get_arg(3) : new Vote(); - $vote ??= new Vote(); + $vote = 3 < \func_num_args() ? func_get_arg(3) : null; $result = VoterInterface::ACCESS_ABSTAIN; $roles = $this->extractRoles($token); $missingRoles = []; @@ -44,7 +43,7 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes/* $result = VoterInterface::ACCESS_DENIED; if (\in_array($attribute, $roles, true)) { - $vote->reasons[] = \sprintf('The user has %s.', $attribute); + $vote?->addReason(\sprintf('The user has %s.', $attribute)); return VoterInterface::ACCESS_GRANTED; } @@ -53,7 +52,7 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes/* } if (VoterInterface::ACCESS_DENIED === $result) { - $vote->reasons[] = \sprintf('The user doesn\'t have%s %s.', 1 < \count($missingRoles) ? ' any of' : '', implode(', ', $missingRoles)); + $vote?->addReason(\sprintf('The user doesn\'t have%s %s.', 1 < \count($missingRoles) ? ' any of' : '', implode(', ', $missingRoles))); } return $result; diff --git a/Authorization/Voter/TraceableVoter.php b/Authorization/Voter/TraceableVoter.php index 47572797..ec926063 100644 --- a/Authorization/Voter/TraceableVoter.php +++ b/Authorization/Voter/TraceableVoter.php @@ -32,9 +32,9 @@ public function __construct( public function vote(TokenInterface $token, mixed $subject, array $attributes, ?Vote $vote = null): int { - $result = $this->voter->vote($token, $subject, $attributes, $vote ??= new Vote()); + $result = $this->voter->vote($token, $subject, $attributes, $vote); - $this->eventDispatcher->dispatch(new VoteEvent($this->voter, $subject, $attributes, $result, $vote->reasons), 'debug.security.authorization.vote'); + $this->eventDispatcher->dispatch(new VoteEvent($this->voter, $subject, $attributes, $result, $vote->reasons ?? []), 'debug.security.authorization.vote'); return $result; } diff --git a/Authorization/Voter/Voter.php b/Authorization/Voter/Voter.php index 3d7fd9e2..55930def 100644 --- a/Authorization/Voter/Voter.php +++ b/Authorization/Voter/Voter.php @@ -29,10 +29,9 @@ abstract class Voter implements VoterInterface, CacheableVoterInterface */ public function vote(TokenInterface $token, mixed $subject, array $attributes/* , ?Vote $vote = null */): int { - $vote = 3 < \func_num_args() ? func_get_arg(3) : new Vote(); - $vote ??= new Vote(); + $vote = 3 < \func_num_args() ? func_get_arg(3) : null; // abstain vote by default in case none of the attributes are supported - $vote->result = self::ACCESS_ABSTAIN; + $voteResult = self::ACCESS_ABSTAIN; foreach ($attributes as $attribute) { try { @@ -48,15 +47,27 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes/* } // as soon as at least one attribute is supported, default is to deny access - $vote->result = self::ACCESS_DENIED; + $voteResult = self::ACCESS_DENIED; + + if (null !== $vote) { + $vote->result = $voteResult; + } if ($this->voteOnAttribute($attribute, $subject, $token, $vote)) { // grant access as soon as at least one attribute returns a positive response - return $vote->result = self::ACCESS_GRANTED; + if (null !== $vote) { + $vote->result = self::ACCESS_GRANTED; + } + + return self::ACCESS_GRANTED; } } - return $vote->result; + if (null !== $vote) { + $vote->result = $voteResult; + } + + return $voteResult; } /** diff --git a/Tests/Authorization/Voter/VoterTest.php b/Tests/Authorization/Voter/VoterTest.php index a8f87e09..eaada306 100644 --- a/Tests/Authorization/Voter/VoterTest.php +++ b/Tests/Authorization/Voter/VoterTest.php @@ -33,35 +33,51 @@ public static function getTests(): array return [ [$voter, ['EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute and class are supported and attribute grants access'], + [$voter, ['EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute and class are supported and attribute grants access', new Vote()], [$voter, ['CREATE'], VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if attribute and class are supported and attribute does not grant access'], + [$voter, ['CREATE'], VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if attribute and class are supported and attribute does not grant access', new Vote()], [$voter, ['DELETE', 'EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if one attribute is supported and grants access'], + [$voter, ['DELETE', 'EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if one attribute is supported and grants access', new Vote()], [$voter, ['DELETE', 'CREATE'], VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if one attribute is supported and denies access'], + [$voter, ['DELETE', 'CREATE'], VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if one attribute is supported and denies access', new Vote()], [$voter, ['CREATE', 'EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if one attribute grants access'], + [$voter, ['CREATE', 'EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if one attribute grants access', new Vote()], [$voter, ['DELETE'], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attribute is supported'], + [$voter, ['DELETE'], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attribute is supported', new Vote()], [$voter, ['EDIT'], VoterInterface::ACCESS_ABSTAIN, new class {}, 'ACCESS_ABSTAIN if class is not supported'], + [$voter, ['EDIT'], VoterInterface::ACCESS_ABSTAIN, new class {}, 'ACCESS_ABSTAIN if class is not supported', new Vote()], [$voter, ['EDIT'], VoterInterface::ACCESS_ABSTAIN, null, 'ACCESS_ABSTAIN if object is null'], + [$voter, ['EDIT'], VoterInterface::ACCESS_ABSTAIN, null, 'ACCESS_ABSTAIN if object is null', new Vote()], [$voter, [], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attributes were provided'], + [$voter, [], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attributes were provided', new Vote()], [$voter, [new StringableAttribute()], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute and class are supported and attribute grants access'], + [$voter, [new StringableAttribute()], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute and class are supported and attribute grants access', new Vote()], [$voter, [new \stdClass()], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if attributes were not strings'], + [$voter, [new \stdClass()], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if attributes were not strings', new Vote()], [$integerVoter, [42], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute is an integer'], + [$integerVoter, [42], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute is an integer', new Vote()], ]; } /** * @dataProvider getTests */ - public function testVote(VoterInterface $voter, array $attributes, $expectedVote, $object, $message) + public function testVote(VoterInterface $voter, array $attributes, $expectedVote, $object, $message, ?Vote $vote = null) { - $this->assertEquals($expectedVote, $voter->vote($this->token, $object, $attributes), $message); + $this->assertSame($expectedVote, $voter->vote($this->token, $object, $attributes, $vote), $message); + + if (null !== $vote) { + self::assertSame($expectedVote, $vote->result); + } } public function testVoteWithTypeError() From 527780a0482e592530174ca90e6189f64cdf6569 Mon Sep 17 00:00:00 2001 From: Abdelhakim ABOULHAJ Date: Wed, 28 May 2025 15:48:55 +0100 Subject: [PATCH 51/52] doc: update UserInterface header comments --- User/UserInterface.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/User/UserInterface.php b/User/UserInterface.php index ef22340a..a543c35c 100644 --- a/User/UserInterface.php +++ b/User/UserInterface.php @@ -15,9 +15,7 @@ * Represents the interface that all user classes must implement. * * This interface is useful because the authentication layer can deal with - * the object through its lifecycle, using the object to get the hashed - * password (for checking against a submitted password), assigning roles - * and so on. + * the object through its lifecycle, assigning roles and so on. * * Regardless of how your users are loaded or where they come from (a database, * configuration, web service, etc.), you will have a class that implements From 577a5a3693a7b4fc11057aa8fb235a95c9deebfd Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 4 Jun 2025 14:09:48 +0200 Subject: [PATCH 52/52] [Security] Keep roles when serializing tokens --- Authentication/Token/AbstractToken.php | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/Authentication/Token/AbstractToken.php b/Authentication/Token/AbstractToken.php index b2e18a29..683e46d4 100644 --- a/Authentication/Token/AbstractToken.php +++ b/Authentication/Token/AbstractToken.php @@ -32,16 +32,12 @@ abstract class AbstractToken implements TokenInterface, \Serializable */ public function __construct(array $roles = []) { - $this->roleNames = []; - - foreach ($roles as $role) { - $this->roleNames[] = (string) $role; - } + $this->roleNames = $roles; } public function getRoleNames(): array { - return $this->roleNames ??= self::__construct($this->user->getRoles()) ?? $this->roleNames; + return $this->roleNames ??= $this->user?->getRoles() ?? []; } public function getUserIdentifier(): string @@ -90,13 +86,7 @@ public function eraseCredentials(): void */ public function __serialize(): array { - $data = [$this->user, true, null, $this->attributes]; - - if (!$this->user instanceof EquatableInterface) { - $data[] = $this->roleNames; - } - - return $data; + return [$this->user, true, null, $this->attributes, $this->getRoleNames()]; } /** @@ -160,12 +150,7 @@ public function __toString(): string $class = static::class; $class = substr($class, strrpos($class, '\\') + 1); - $roles = []; - foreach ($this->roleNames as $role) { - $roles[] = $role; - } - - return \sprintf('%s(user="%s", roles="%s")', $class, $this->getUserIdentifier(), implode(', ', $roles)); + return \sprintf('%s(user="%s", roles="%s")', $class, $this->getUserIdentifier(), implode(', ', $this->getRoleNames())); } /**