diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bec670bbbb..63e06b38516 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 2.6.3 +* Security: Use a `NullToken` when using the new authenticator manager in the resource access checker (#4067) * Mercure: Do not use data in options when deleting (#4056) * Doctrine: Support for foreign identifiers * SchemaFactory: Allow generating documentation when property and method start from "is" (property `isActive` and method `isActive`) diff --git a/src/Bridge/Symfony/Bundle/ApiPlatformBundle.php b/src/Bridge/Symfony/Bundle/ApiPlatformBundle.php index 560b61d80c8..da038d8324b 100644 --- a/src/Bridge/Symfony/Bundle/ApiPlatformBundle.php +++ b/src/Bridge/Symfony/Bundle/ApiPlatformBundle.php @@ -14,6 +14,7 @@ namespace ApiPlatform\Core\Bridge\Symfony\Bundle; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\AnnotationFilterPass; +use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\AuthenticatorManagerPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\DataProviderPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\DeprecateMercurePublisherPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\ElasticsearchClientPass; @@ -53,5 +54,6 @@ public function build(ContainerBuilder $container) $container->addCompilerPass(new DeprecateMercurePublisherPass()); $container->addCompilerPass(new MetadataAwareNameConverterPass()); $container->addCompilerPass(new TestClientPass()); + $container->addCompilerPass(new AuthenticatorManagerPass()); } } diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/AuthenticatorManagerPass.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/AuthenticatorManagerPass.php new file mode 100644 index 00000000000..6df6b0cde60 --- /dev/null +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/AuthenticatorManagerPass.php @@ -0,0 +1,37 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler; + +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\ContainerBuilder; + +/** + * Checks if the new authenticator manager exists. + * + * @internal + * + * @author Alan Poulain + */ +final class AuthenticatorManagerPass implements CompilerPassInterface +{ + /** + * {@inheritdoc} + */ + public function process(ContainerBuilder $container): void + { + if ($container->has('security.authenticator.manager')) { + $container->getDefinition('api_platform.security.resource_access_checker')->setArgument(5, false); + } + } +} diff --git a/src/Security/ResourceAccessChecker.php b/src/Security/ResourceAccessChecker.php index 67d8fad1299..23862b49a1a 100644 --- a/src/Security/ResourceAccessChecker.php +++ b/src/Security/ResourceAccessChecker.php @@ -15,6 +15,7 @@ use Symfony\Component\ExpressionLanguage\ExpressionLanguage; use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface; +use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; @@ -33,14 +34,16 @@ final class ResourceAccessChecker implements ResourceAccessCheckerInterface private $roleHierarchy; private $tokenStorage; private $authorizationChecker; + private $exceptionOnNoToken; - public function __construct(ExpressionLanguage $expressionLanguage = null, AuthenticationTrustResolverInterface $authenticationTrustResolver = null, RoleHierarchyInterface $roleHierarchy = null, TokenStorageInterface $tokenStorage = null, AuthorizationCheckerInterface $authorizationChecker = null) + public function __construct(ExpressionLanguage $expressionLanguage = null, AuthenticationTrustResolverInterface $authenticationTrustResolver = null, RoleHierarchyInterface $roleHierarchy = null, TokenStorageInterface $tokenStorage = null, AuthorizationCheckerInterface $authorizationChecker = null, bool $exceptionOnNoToken = true) { $this->expressionLanguage = $expressionLanguage; $this->authenticationTrustResolver = $authenticationTrustResolver; $this->roleHierarchy = $roleHierarchy; $this->tokenStorage = $tokenStorage; $this->authorizationChecker = $authorizationChecker; + $this->exceptionOnNoToken = $exceptionOnNoToken; } public function isGranted(string $resourceClass, string $expression, array $extraVariables = []): bool @@ -48,6 +51,15 @@ public function isGranted(string $resourceClass, string $expression, array $extr if (null === $this->tokenStorage || null === $this->authenticationTrustResolver) { throw new \LogicException('The "symfony/security" library must be installed to use the "security" attribute.'); } + if (null === $token = $this->tokenStorage->getToken()) { + if ($this->exceptionOnNoToken) { + throw new \LogicException('The current token must be set to use the "security" attribute (is the URL behind a firewall?).'); + } + + if (class_exists(NullToken::class)) { + $token = new NullToken(); + } + } if (null === $this->expressionLanguage) { throw new \LogicException('The "symfony/expression-language" library must be installed to use the "security" attribute.'); } @@ -57,7 +69,7 @@ public function isGranted(string $resourceClass, string $expression, array $extr 'auth_checker' => $this->authorizationChecker, // needed for the is_granted expression function ]); - if ($token = $this->tokenStorage->getToken()) { + if ($token) { $variables = array_merge($variables, $this->getVariables($token)); } diff --git a/tests/Bridge/Symfony/Bundle/ApiPlatformBundleTest.php b/tests/Bridge/Symfony/Bundle/ApiPlatformBundleTest.php index 5ea6458148c..ac043f594ba 100644 --- a/tests/Bridge/Symfony/Bundle/ApiPlatformBundleTest.php +++ b/tests/Bridge/Symfony/Bundle/ApiPlatformBundleTest.php @@ -15,6 +15,7 @@ use ApiPlatform\Core\Bridge\Symfony\Bundle\ApiPlatformBundle; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\AnnotationFilterPass; +use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\AuthenticatorManagerPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\DataProviderPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\DeprecateMercurePublisherPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\ElasticsearchClientPass; @@ -50,6 +51,7 @@ public function testBuild() $containerProphecy->addCompilerPass(Argument::type(DeprecateMercurePublisherPass::class))->shouldBeCalled(); $containerProphecy->addCompilerPass(Argument::type(MetadataAwareNameConverterPass::class))->shouldBeCalled(); $containerProphecy->addCompilerPass(Argument::type(TestClientPass::class))->shouldBeCalled(); + $containerProphecy->addCompilerPass(Argument::type(AuthenticatorManagerPass::class))->shouldBeCalled(); $bundle = new ApiPlatformBundle(); $bundle->build($containerProphecy->reveal()); diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/Compiler/AuthenticatorManagerPassTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/Compiler/AuthenticatorManagerPassTest.php new file mode 100644 index 00000000000..eab57b2812a --- /dev/null +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/Compiler/AuthenticatorManagerPassTest.php @@ -0,0 +1,55 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Bridge\Symfony\Bundle\DependencyInjection\Compiler; + +use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\AuthenticatorManagerPass; +use PHPUnit\Framework\TestCase; +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; + +final class AuthenticatorManagerPassTest extends TestCase +{ + private $containerBuilderProphecy; + private $authenticatorManagerPass; + + protected function setUp(): void + { + $this->containerBuilderProphecy = $this->prophesize(ContainerBuilder::class); + $this->authenticatorManagerPass = new AuthenticatorManagerPass(); + } + + public function testConstruct(): void + { + self::assertInstanceOf(CompilerPassInterface::class, $this->authenticatorManagerPass); + } + + public function testProcessWithoutAuthenticatorManager(): void + { + $this->containerBuilderProphecy->has('security.authenticator.manager')->willReturn(false); + $this->containerBuilderProphecy->getDefinition('api_platform.security.resource_access_checker')->shouldNotBeCalled(); + + $this->authenticatorManagerPass->process($this->containerBuilderProphecy->reveal()); + } + + public function testProcess(): void + { + $this->containerBuilderProphecy->has('security.authenticator.manager')->willReturn(true); + $authenticatorManagerDefinitionProphecy = $this->prophesize(Definition::class); + $this->containerBuilderProphecy->getDefinition('api_platform.security.resource_access_checker')->willReturn($authenticatorManagerDefinitionProphecy->reveal()); + $authenticatorManagerDefinitionProphecy->setArgument(5, false)->shouldBeCalledOnce(); + + $this->authenticatorManagerPass->process($this->containerBuilderProphecy->reveal()); + } +} diff --git a/tests/Security/ResourceAccessCheckerTest.php b/tests/Security/ResourceAccessCheckerTest.php index b22314290cc..6aeaa41df47 100644 --- a/tests/Security/ResourceAccessCheckerTest.php +++ b/tests/Security/ResourceAccessCheckerTest.php @@ -23,6 +23,7 @@ use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; /** * @author Kévin Dunglas @@ -85,4 +86,31 @@ public function testExpressionLanguageNotInstalled() $checker = new ResourceAccessChecker(null, $authenticationTrustResolverProphecy->reveal(), null, $tokenStorageProphecy->reveal()); $checker->isGranted(Dummy::class, 'is_granted("ROLE_ADMIN")'); } + + public function testNotBehindAFirewall() + { + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('The current token must be set to use the "security" attribute (is the URL behind a firewall?).'); + + $authenticationTrustResolverProphecy = $this->prophesize(AuthenticationTrustResolverInterface::class); + $tokenStorageProphecy = $this->prophesize(TokenStorageInterface::class); + + $checker = new ResourceAccessChecker(null, $authenticationTrustResolverProphecy->reveal(), null, $tokenStorageProphecy->reveal()); + $checker->isGranted(Dummy::class, 'is_granted("ROLE_ADMIN")'); + } + + public function testWithoutAuthenticationTokenAndExceptionOnNoTokenIsFalse() + { + $expressionLanguageProphecy = $this->prophesize(ExpressionLanguage::class); + $expressionLanguageProphecy->evaluate('is_granted("ROLE_ADMIN")', Argument::type('array'))->willReturn(true)->shouldBeCalled(); + + $authenticationTrustResolverProphecy = $this->prophesize(AuthenticationTrustResolverInterface::class); + $authorizationCheckerProphecy = $this->prophesize(AuthorizationCheckerInterface::class); + $tokenStorageProphecy = $this->prophesize(TokenStorageInterface::class); + + $tokenStorageProphecy->getToken()->willReturn(null); + + $checker = new ResourceAccessChecker($expressionLanguageProphecy->reveal(), $authenticationTrustResolverProphecy->reveal(), null, $tokenStorageProphecy->reveal(), $authorizationCheckerProphecy->reveal(), false); + self::assertTrue($checker->isGranted(Dummy::class, 'is_granted("ROLE_ADMIN")')); + } }