diff --git a/src/Symfony/Security/State/AccessCheckerProvider.php b/src/Symfony/Security/State/AccessCheckerProvider.php index af86387c1f9..7b2065af614 100644 --- a/src/Symfony/Security/State/AccessCheckerProvider.php +++ b/src/Symfony/Security/State/AccessCheckerProvider.php @@ -85,7 +85,14 @@ public function provide(Operation $operation, array $uriVariables = [], array $c ]; } - if ('pre_read' === $this->event && $this->resourceAccessChecker instanceof ObjectVariableCheckerInterface && $this->resourceAccessChecker->usesObjectVariable($isGranted, $resourceAccessCheckerContext)) { + // Skip pre_read optimization when object is used via granted (or usage is not predicatable) + if ( + 'pre_read' === $this->event + && ( + !$this->resourceAccessChecker instanceof ObjectVariableCheckerInterface + || $this->resourceAccessChecker->usesObjectVariable($isGranted, $resourceAccessCheckerContext) + ) + ) { return $this->decorated->provide($operation, $uriVariables, $context); } diff --git a/tests/Symfony/Security/State/AccessCheckerProviderTest.php b/tests/Symfony/Security/State/AccessCheckerProviderTest.php index c4545c4a258..7e83cc49be4 100644 --- a/tests/Symfony/Security/State/AccessCheckerProviderTest.php +++ b/tests/Symfony/Security/State/AccessCheckerProviderTest.php @@ -18,6 +18,7 @@ use ApiPlatform\Metadata\ResourceAccessCheckerInterface; use ApiPlatform\State\ProviderInterface; use ApiPlatform\Symfony\Security\Exception\AccessDeniedException; +use ApiPlatform\Symfony\Security\ObjectVariableCheckerInterface; use ApiPlatform\Symfony\Security\State\AccessCheckerProvider; use ApiPlatform\Tests\Fixtures\DummyEntity; use PHPUnit\Framework\TestCase; @@ -61,6 +62,44 @@ public function testCheckAccessWithEventPostValidate(): void $accessChecker->provide($operation, [], []); } + public function testPreReadSkipsSecurityWhenResourceAccessCheckerIsDecorated(): void + { + $obj = new \stdClass(); + $operation = new Get(class: DummyEntity::class, security: 'is_granted("ROLE_ADMIN")'); + $decorated = $this->createMock(ProviderInterface::class); + $decorated->expects($this->once())->method('provide')->willReturn($obj); + $resourceAccessChecker = $this->createMock(ResourceAccessCheckerInterface::class); + $resourceAccessChecker->expects($this->never())->method('isGranted'); + $accessChecker = new AccessCheckerProvider($decorated, $resourceAccessChecker, 'pre_read'); + $this->assertSame($obj, $accessChecker->provide($operation, [], [])); + } + + public function testPreReadChecksSecurityWhenObjectVariableIsNotUsed(): void + { + $obj = new \stdClass(); + $operation = new Get(class: DummyEntity::class, security: 'is_granted("ROLE_ADMIN")'); + $decorated = $this->createMock(ProviderInterface::class); + $decorated->expects($this->once())->method('provide')->willReturn($obj); + $resourceAccessChecker = $this->createMock(ResourceAccessCheckerWithObjectVariableInterface::class); + $resourceAccessChecker->method('usesObjectVariable')->willReturn(false); + $resourceAccessChecker->expects($this->once())->method('isGranted')->with(DummyEntity::class, 'is_granted("ROLE_ADMIN")', ['object' => null, 'previous_object' => null, 'request' => null])->willReturn(true); + $accessChecker = new AccessCheckerProvider($decorated, $resourceAccessChecker, 'pre_read'); + $this->assertSame($obj, $accessChecker->provide($operation, [], [])); + } + + public function testPreReadSkipsSecurityWhenObjectVariableIsUsed(): void + { + $obj = new \stdClass(); + $operation = new Get(class: DummyEntity::class, security: 'is_granted("ROLE_ADMIN") and object.owner == user'); + $decorated = $this->createMock(ProviderInterface::class); + $decorated->expects($this->once())->method('provide')->willReturn($obj); + $resourceAccessChecker = $this->createMock(ResourceAccessCheckerWithObjectVariableInterface::class); + $resourceAccessChecker->method('usesObjectVariable')->willReturn(true); + $resourceAccessChecker->expects($this->never())->method('isGranted'); + $accessChecker = new AccessCheckerProvider($decorated, $resourceAccessChecker, 'pre_read'); + $this->assertSame($obj, $accessChecker->provide($operation, [], [])); + } + public function testCheckAccessDenied(): void { $this->expectException(AccessDeniedException::class); @@ -91,3 +130,7 @@ public function testCheckAccessDeniedWithGraphQl(): void $accessChecker->provide($operation, [], []); } } + +interface ResourceAccessCheckerWithObjectVariableInterface extends ResourceAccessCheckerInterface, ObjectVariableCheckerInterface +{ +}