Merge f6d579f into 2c31621

api-platform · Feb 16, 2021 · e679828 · e679828
2 parents 2c31621 + f6d579f
commit e679828
Show file tree

Hide file tree

Showing 7 changed files with 139 additions and 2 deletions.
diff --git 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
@@ -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
@@ -0,0 +1,37 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <dunglas@gmail.com>
+ *
+ * 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 <contact@alanpoulain.eu>
+ */
+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
@@ -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,21 +34,32 @@ 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
     {
         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
@@ -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
@@ -0,0 +1,55 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <dunglas@gmail.com>
+ *
+ * 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
@@ -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 <dunglas@gmail.com>
@@ -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")'));
+    }
 }