diff --git a/Authentication/Token/AbstractToken.php b/Authentication/Token/AbstractToken.php index b2e18a2..683e46d 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())); } /** diff --git a/Authorization/Voter/AuthenticatedVoter.php b/Authorization/Voter/AuthenticatedVoter.php index 1403aaa..3ab6b92 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 03a9f75..4fb5502 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 35d727a..719aae7 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 46c08d1..2225e8d 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 4757279..ec92606 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 3d7fd9e..55930de 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 a8f87e0..eaada30 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() diff --git a/User/UserInterface.php b/User/UserInterface.php index 1205212..24c0581 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