diff --git a/Authentication/Token/AbstractToken.php b/Authentication/Token/AbstractToken.php index 67d992ce..b2e18a29 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 @@ -58,8 +59,15 @@ public function setUser(UserInterface $user): void $this->user = $user; } + /** + * Removes sensitive information from the token. + * + * @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::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(); } @@ -82,7 +90,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 +117,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/Authentication/Token/NullToken.php b/Authentication/Token/NullToken.php index 9c2e4892..cb2bc0fd 100644 --- a/Authentication/Token/NullToken.php +++ b/Authentication/Token/NullToken.php @@ -43,8 +43,15 @@ public function getUserIdentifier(): string return ''; } + /** + * @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/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/TokenInterface.php b/Authentication/Token/TokenInterface.php index 1e67b1e5..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 */ @@ -56,6 +59,8 @@ public function setUser(UserInterface $user): void; /** * Removes sensitive information from the token. + * + * @deprecated since Symfony 7.3; erase credentials using the "__serialize()" method instead */ public function eraseCredentials(): void; 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..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,22 +25,44 @@ * @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( 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(); + $token = end($this->tokenStack) ?: $this->tokenStorage->getToken(); if (!$token || !$token->getUser()) { $token = new NullToken(); } + $accessDecision ??= end($this->accessDecisionStack) ?: new AccessDecision(); + $this->accessDecisionStack[] = $accessDecision; + + try { + return $accessDecision->isGranted = $this->accessDecisionManager->decide($token, [$attribute], $subject, $accessDecision); + } finally { + array_pop($this->accessDecisionStack); + } + } - return $this->accessDecisionManager->decide($token, [$attribute], $subject); + 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/AuthorizationCheckerInterface.php b/Authorization/AuthorizationCheckerInterface.php index 6f5a6022..848b17ee 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; 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): bool; + public function isGranted(mixed $attribute, mixed $subject = null/* , ?AccessDecision $accessDecision = null */): bool; } 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()); 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/UserAuthorizationCheckerInterface.php b/Authorization/UserAuthorizationCheckerInterface.php new file mode 100644 index 00000000..15e5b4d4 --- /dev/null +++ b/Authorization/UserAuthorizationCheckerInterface.php @@ -0,0 +1,30 @@ + + * + * 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) + * @param AccessDecision|null $accessDecision Should be used to explain the decision + */ + 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 a0011868..1403aaaa 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, @@ -38,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; } @@ -54,32 +64,51 @@ 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 - && $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/ClosureVoter.php b/Authorization/Voter/ClosureVoter.php new file mode 100644 index 00000000..03a9f757 --- /dev/null +++ b/Authorization/Voter/ClosureVoter.php @@ -0,0 +1,71 @@ + + * + * 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\Token\TokenInterface; +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. + * + * @see IsGranted doc for the complete closure signature. + * + * @author Alexandre Daubois + */ +final class ClosureVoter implements CacheableVoterInterface +{ + public function __construct( + private AuthorizationCheckerInterface $authorizationChecker, + ) { + } + + 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(); + $context = new IsGrantedContext($token, $token->getUser(), $this->authorizationChecker); + $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($context, $subject)) { + $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/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 7cf09c70..12806416 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,17 @@ CHANGELOG ========= +7.3 +--- + + * 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 + * 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/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/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 cc1357a1..3972b1cd 100644 --- a/Tests/Authentication/Token/AbstractTokenTest.php +++ b/Tests/Authentication/Token/AbstractTokenTest.php @@ -12,12 +12,16 @@ namespace Symfony\Component\Security\Core\Tests\Authentication\Token; use PHPUnit\Framework\TestCase; +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; use Symfony\Component\Security\Core\User\UserInterface; class AbstractTokenTest extends TestCase { + use ExpectUserDeprecationMessageTrait; + /** * @dataProvider provideUsers */ @@ -33,6 +37,9 @@ public static function provideUsers() yield [new InMemoryUser('fabien', null), 'fabien']; } + /** + * @group legacy + */ public function testEraseCredentials() { $token = new ConcreteToken(['ROLE_FOO']); @@ -41,6 +48,8 @@ public function testEraseCredentials() $user->expects($this->once())->method('eraseCredentials'); $token->setUser($user); + $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/Authentication/Token/Fixtures/CustomUser.php b/Tests/Authentication/Token/Fixtures/CustomUser.php index 99302032..d4f91de1 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 @@ -32,11 +36,7 @@ public function getPassword(): ?string return null; } - public function getSalt(): ?string - { - return null; - } - + #[\Deprecated] public function eraseCredentials(): void { } 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/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/AuthenticatedVoterTest.php b/Tests/Authorization/Voter/AuthenticatedVoterTest.php index ed894b3a..b5e0bf42 100644 --- a/Tests/Authorization/Voter/AuthenticatedVoterTest.php +++ b/Tests/Authorization/Voter/AuthenticatedVoterTest.php @@ -15,10 +15,12 @@ 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\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 class($user->getRoles()) extends AbstractToken implements OfflineTokenInterface {}; + } + return new NullToken(); } } diff --git a/Tests/Authorization/Voter/ClosureVoterTest.php b/Tests/Authorization/Voter/ClosureVoterTest.php new file mode 100644 index 00000000..7a22f2d4 --- /dev/null +++ b/Tests/Authorization/Voter/ClosureVoterTest.php @@ -0,0 +1,89 @@ + + * + * 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\Token\TokenInterface; +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; + + protected function setUp(): void + { + $this->voter = new ClosureVoter( + $this->createMock(AuthorizationCheckerInterface::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 () => 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 () => 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 (IsGrantedContext $context, \stdClass $subject) use ($outerSubject) { + $this->assertSame($outerSubject, $subject); + + return 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/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); 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 { } } diff --git a/Tests/User/InMemoryUserTest.php b/Tests/User/InMemoryUserTest.php index 0e64bce5..f06e98c3 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\ExpectUserDeprecationMessageTrait; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Core\User\UserInterface; class InMemoryUserTest extends TestCase { + use ExpectUserDeprecationMessageTrait; + public function testConstructorException() { $this->expectException(\InvalidArgumentException::class); @@ -53,9 +56,13 @@ public function testIsEnabled() $this->assertFalse($user->isEnabled()); } + /** + * @group legacy + */ public function testEraseCredentials() { $user = new InMemoryUser('fabien', 'superpass'); + $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()); } 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/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/InMemoryUser.php b/User/InMemoryUser.php index b14bc077..7bed183a 100644 --- a/User/InMemoryUser.php +++ b/User/InMemoryUser.php @@ -74,8 +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 { + 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/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 + { + } +} 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..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 */ 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; } 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; } diff --git a/User/UserInterface.php b/User/UserInterface.php index e6078399..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 @@ -51,6 +54,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. + * + * @deprecated since Symfony 7.3, erase credentials using the "__serialize()" method instead */ public function eraseCredentials(): void;