From 0e709bb5b1c4d61a508789d5c7d7cbd75aab77cf Mon Sep 17 00:00:00 2001 From: Giorgio Premi Date: Tue, 31 Mar 2026 12:42:26 +0200 Subject: [PATCH 1/2] fix(symfony): security regression when ResourceAccessChecker is decorated (#7896) Commit 359a128cd56934aeb3aefc13040fcd1206907157 introduced a regression when ResourceAccessChecker is decorated, and security/securityPostDenormalize are using object in is_granted expression. The issue arise since AccessCheckerProvider violates the Liskov substitution principle by assuming that if the (previously unknown) interface ObjectVariableCheckerInterface is not defined, then the pre_read optimization can be used without an object instance. --- src/Symfony/Security/State/AccessCheckerProvider.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) 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); } From e6493c2d430819abbfdeaa76b590c6961187e96d Mon Sep 17 00:00:00 2001 From: soyuka Date: Mon, 6 Apr 2026 08:33:49 +0200 Subject: [PATCH 2/2] test --- .../State/AccessCheckerProviderTest.php | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) 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 +{ +}