From 7d0f9175be562871b5d692ba8a6728e31fc819ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 15 Jul 2024 17:18:28 +0200 Subject: [PATCH] Refactor StoredObjectVoterTest to improve testing logic The existing StoredObjectVoter test logic was reworked to utilize UsernamePasswordToken and Security mock objects instead of defining its own token. This change improves the testing for different scenarios such as unsupported attributes and cases where role voters cannot see the stored object. Also, the redundancy in the test case provider was removed, which leads to cleaner and more maintainable code. --- .../Authorization/StoredObjectVoterTest.php | 132 +++++++++--------- 1 file changed, 65 insertions(+), 67 deletions(-) diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Security/Authorization/StoredObjectVoterTest.php b/src/Bundle/ChillDocStoreBundle/Tests/Security/Authorization/StoredObjectVoterTest.php index 92c928681..a113cfb7b 100644 --- a/src/Bundle/ChillDocStoreBundle/Tests/Security/Authorization/StoredObjectVoterTest.php +++ b/src/Bundle/ChillDocStoreBundle/Tests/Security/Authorization/StoredObjectVoterTest.php @@ -14,11 +14,13 @@ namespace Chill\DocStoreBundle\Tests\Security\Authorization; use Chill\DocStoreBundle\Entity\StoredObject; use Chill\DocStoreBundle\Security\Authorization\StoredObjectRoleEnum; use Chill\DocStoreBundle\Security\Authorization\StoredObjectVoter; -use Chill\DocStoreBundle\Security\Guard\DavTokenAuthenticationEventSubscriber; +use Chill\DocStoreBundle\Security\Authorization\StoredObjectVoterInterface; +use Chill\MainBundle\Entity\User; use PHPUnit\Framework\TestCase; -use Prophecy\PhpUnit\ProphecyTrait; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; +use Symfony\Component\Security\Core\Security; /** * @internal @@ -27,97 +29,93 @@ use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; */ class StoredObjectVoterTest extends TestCase { - use ProphecyTrait; - /** * @dataProvider provideDataVote */ - public function testVote(TokenInterface $token, ?object $subject, string $attribute, mixed $expected): void + public function testVote(array $storedObjectVotersDefinition, object $subject, string $attribute, bool $fallbackSecurityExpected, bool $securityIsGrantedResult, mixed $expected): void { - $voter = new StoredObjectVoter(); + $storedObjectVoters = array_map(fn (array $definition) => $this->buildStoredObjectVoter($definition[0], $definition[1], $definition[2]), $storedObjectVotersDefinition); + $token = new UsernamePasswordToken(new User(), 'chill_main', ['ROLE_USER']); + + $security = $this->createMock(Security::class); + $security->expects($fallbackSecurityExpected ? $this->atLeastOnce() : $this->never()) + ->method('isGranted') + ->with($this->logicalOr($this->identicalTo('ROLE_USER'), $this->identicalTo('ROLE_ADMIN'))) + ->willReturn($securityIsGrantedResult); + + $voter = new StoredObjectVoter($security, $storedObjectVoters); self::assertEquals($expected, $voter->vote($token, $subject, [$attribute])); } - public function provideDataVote(): iterable + private function buildStoredObjectVoter(bool $supportsIsCalled, bool $supports, bool $voteOnAttribute): StoredObjectVoterInterface + { + $storedObjectVoter = $this->createMock(StoredObjectVoterInterface::class); + $storedObjectVoter->expects($supportsIsCalled ? $this->once() : $this->never())->method('supports') + ->with(self::isInstanceOf(StoredObjectRoleEnum::class), $this->isInstanceOf(StoredObject::class)) + ->willReturn($supports); + $storedObjectVoter->expects($supportsIsCalled && $supports ? $this->once() : $this->never())->method('voteOnAttribute') + ->with(self::isInstanceOf(StoredObjectRoleEnum::class), $this->isInstanceOf(StoredObject::class), $this->isInstanceOf(TokenInterface::class)) + ->willReturn($voteOnAttribute); + + return $storedObjectVoter; + } + + public static function provideDataVote(): iterable { yield [ - $this->buildToken(StoredObjectRoleEnum::EDIT, new StoredObject()), + // we try with something else than a SToredObject, the voter should abstain + [[false, false, false]], new \stdClass(), 'SOMETHING', + false, + false, VoterInterface::ACCESS_ABSTAIN, ]; - yield [ - $this->buildToken(StoredObjectRoleEnum::EDIT, $so = new StoredObject()), - $so, + // we try with an unsupported attribute, the voter must abstain + [[false, false, false]], + new StoredObject(), 'SOMETHING', + false, + false, VoterInterface::ACCESS_ABSTAIN, ]; - yield [ - $this->buildToken(StoredObjectRoleEnum::EDIT, $so = new StoredObject()), - $so, - StoredObjectRoleEnum::SEE->value, - VoterInterface::ACCESS_GRANTED, - ]; - - yield [ - $this->buildToken(StoredObjectRoleEnum::EDIT, $so = new StoredObject()), - $so, - StoredObjectRoleEnum::EDIT->value, - VoterInterface::ACCESS_GRANTED, - ]; - - yield [ - $this->buildToken(StoredObjectRoleEnum::SEE, $so = new StoredObject()), - $so, - StoredObjectRoleEnum::EDIT->value, - VoterInterface::ACCESS_DENIED, - ]; - - yield [ - $this->buildToken(StoredObjectRoleEnum::SEE, $so = new StoredObject()), - $so, - StoredObjectRoleEnum::SEE->value, - VoterInterface::ACCESS_GRANTED, - ]; - - yield [ - $this->buildToken(null, null), + // happy scenario: there is a role voter + [[true, true, true]], new StoredObject(), StoredObjectRoleEnum::SEE->value, - VoterInterface::ACCESS_DENIED, + false, + false, + VoterInterface::ACCESS_GRANTED, ]; - yield [ - $this->buildToken(null, null), + // there is a role voter, but not allowed to see the stored object + [[true, true, false]], new StoredObject(), StoredObjectRoleEnum::SEE->value, + false, + false, VoterInterface::ACCESS_DENIED, ]; - } - - private function buildToken(?StoredObjectRoleEnum $storedObjectRoleEnum = null, ?StoredObject $storedObject = null): TokenInterface - { - $token = $this->prophesize(TokenInterface::class); - - if (null !== $storedObjectRoleEnum) { - $token->hasAttribute(DavTokenAuthenticationEventSubscriber::ACTIONS)->willReturn(true); - $token->getAttribute(DavTokenAuthenticationEventSubscriber::ACTIONS)->willReturn($storedObjectRoleEnum); - } else { - $token->hasAttribute(DavTokenAuthenticationEventSubscriber::ACTIONS)->willReturn(false); - $token->getAttribute(DavTokenAuthenticationEventSubscriber::ACTIONS)->willThrow(new \InvalidArgumentException()); - } - - if (null !== $storedObject) { - $token->hasAttribute(DavTokenAuthenticationEventSubscriber::STORED_OBJECT)->willReturn(true); - $token->getAttribute(DavTokenAuthenticationEventSubscriber::STORED_OBJECT)->willReturn($storedObject->getUuid()->toString()); - } else { - $token->hasAttribute(DavTokenAuthenticationEventSubscriber::STORED_OBJECT)->willReturn(false); - $token->getAttribute(DavTokenAuthenticationEventSubscriber::STORED_OBJECT)->willThrow(new \InvalidArgumentException()); - } - - return $token->reveal(); + yield [ + // there is no role voter, fallback to security, which does not grant access + [[true, false, false]], + new StoredObject(), + StoredObjectRoleEnum::SEE->value, + true, + false, + VoterInterface::ACCESS_DENIED, + ]; + yield [ + // there is no role voter, fallback to security, which does grant access + [[true, false, false]], + new StoredObject(), + StoredObjectRoleEnum::SEE->value, + true, + true, + VoterInterface::ACCESS_GRANTED, + ]; } }