From d14d8727c6ba84c896e500df754c1de541e24155 Mon Sep 17 00:00:00 2001 From: Fran Moreno Date: Tue, 13 Oct 2020 09:08:34 +0200 Subject: [PATCH] Deprecate acl_user_manager without implementing AdminAclUserManagerInterface It also deprecates to not explicitly configuring acl_user_manager service when using ACL because if you use "friendsofsymfony/user-bundle", it adds "fos_user_manager" as "acl_user_manager" automatically. After this, this service MUST implements AdminAclUserManagerInterface allowing us to have an API for retrieving users when using ACL. --- UPGRADE-3.x.md | 48 +++++++++++++ docs/reference/security.rst | 69 ++++++++++++------- src/Controller/CRUDController.php | 18 +++++ .../CheckAclUserManagerCompilerPass.php | 53 ++++++++++++++ .../SonataAdminExtension.php | 11 +++ src/Util/AdminAclUserManagerInterface.php | 25 +++++++ tests/Controller/CRUDControllerTest.php | 24 +++++++ .../CheckAclUserManagerCompilerPassTest.php | 47 +++++++++++++ 8 files changed, 272 insertions(+), 23 deletions(-) create mode 100644 src/DependencyInjection/Compiler/CheckAclUserManagerCompilerPass.php create mode 100644 src/Util/AdminAclUserManagerInterface.php create mode 100644 tests/DependencyInjection/Compiler/CheckAclUserManagerCompilerPassTest.php diff --git a/UPGRADE-3.x.md b/UPGRADE-3.x.md index 2b0c22c4ad..464c3d5454 100644 --- a/UPGRADE-3.x.md +++ b/UPGRADE-3.x.md @@ -1,6 +1,54 @@ UPGRADE 3.x =========== +### Deprecated not configuring `acl_user_manager` and using ACL security handler when `friendsofsymfony/user-bundle` is installed. + +If you are using `friendsofsymfony/user-bundle` and using ACL security handler, you MUST explicitly configure the `acl_user_manager`. + +```yaml +sonata_admin: + security: + acl_user_manager: App\Manager\AclFOSUserManager # this service MUST implement "AdminAclUserManagerInterface" +``` + +### Deprecated configuring `acl_user_manager` with a service that does not implement `AdminAclUserManagerInterface`. + +Given this configuration: +```yaml +sonata_admin: + security: + acl_user_manager: 'App\Manager\AclUserManager' +``` + +`App\Manager\AclUserManager` MUST implement `AdminAclUserManagerInterface`, if you are using `fos_user_manager`, this could +be an example: +```php +userManager = $userManager; + } + + public function findUsers(): iterable + { + return $this->userManager->findUsers(); + } +} +``` + UPGRADE FROM 3.76 to 3.77 ========================= diff --git a/docs/reference/security.rst b/docs/reference/security.rst index 87d17dcc8d..290dab56d5 100644 --- a/docs/reference/security.rst +++ b/docs/reference/security.rst @@ -50,8 +50,9 @@ implementation, you can switch the ``security_handler`` to the role security han Configuration ~~~~~~~~~~~~~ -Only the security handler is required to determine which type of security to use. -The other parameters are set as default, change them if needed. +The security handler is required to determine which type of security to use. +In case of using ACL you MUST set ``acl_user_manager`` parameter, the other other ones +are set as default, change them if needed. Using roles: @@ -80,6 +81,9 @@ Using ACL: security: handler: sonata.admin.security.handler.acl + # this service MUST implement ``Sonata\AdminBundle\Util\AdminAclUserManagerInterface``. + acl_user_manager: App\Manager\AclUserManager + role_admin: ROLE_ADMIN role_super_admin: ROLE_SUPER_ADMIN @@ -305,6 +309,46 @@ User class (in a custom UserBundle). Do it as follows:: } } +If you are going to use ACL, you must create a service implementing +`Sonata\AdminBundle\Util\AdminAclUserManagerInterface`:: + + namespace App\Manager; + + use FOS\UserBundle\Model\UserManagerInterface; + use Sonata\AdminBundle\Util\AdminAclUserManagerInterface; + + final class AclUserManager implements AdminAclUserManagerInterface + { + /** + * @var UserManagerInterface + */ + private $userManager; + + public function __construct(UserManagerInterface $userManager) + { + $this->userManager = $userManager; + } + + public function findUsers(): iterable + { + return $this->userManager->findUsers(); + } + } + +and then configure SonataAdminBundle: + +.. configuration-block:: + + .. code-block:: yaml + + # config/packages/sonata_admin.yaml + + sonata_admin: + security: + handler: sonata.admin.security.handler.acl + acl_user_manager: App\Manager\AclUserManager + # ... + In your ``config/packages/fos_user.yaml`` you then need to put the following: .. configuration-block:: @@ -741,27 +785,6 @@ return an iterable collection of roles:: return new \ArrayIterator($roles); } -Custom user manager -~~~~~~~~~~~~~~~~~~~ - -If your project does not use `FOSUserBundle`, you can globally configure another -service to use when retrieving your users. - -- Create a service with a method called ``findUsers()`` returning an iterable - collection of users -- Update your admin configuration to reference your service name - -.. configuration-block:: - - .. code-block:: yaml - - # config/packages/sonata_admin.yaml - - sonata_admin: - security: - - # the name of your service - acl_user_manager: my_user_manager .. _`SonataUserBundle's documentation area`: https://sonata-project.org/bundles/user/master/doc/reference/installation.html .. _`changing the access decision strategy`: https://symfony.com/doc/4.4/security/voters.html#changing-the-access-decision-strategy diff --git a/src/Controller/CRUDController.php b/src/Controller/CRUDController.php index 9c0a4b2b71..d356d72527 100644 --- a/src/Controller/CRUDController.php +++ b/src/Controller/CRUDController.php @@ -977,6 +977,15 @@ public function aclAction($deprecatedId = null) // NEXT_MAJOR: Remove the unused throw $this->createNotFoundException('ACL are not enabled for this admin'); } + if ($this->container->hasParameter('sonata.admin.security.fos_user_autoconfigured') + && $this->getParameter('sonata.admin.security.fos_user_autoconfigured')) { + @trigger_error(sprintf( + 'Not configuring "acl_user_manager" and using ACL security handler is deprecated since' + .' sonata-project/admin-bundle 3.x and will not work on 4.x. You MUST specify the service name' + .' under "sonata_admin.security.acl_user_manager" option.' + ), E_USER_DEPRECATED); + } + $request = $this->getRequest(); $id = $request->get($this->admin->getIdParameter()); $object = $this->admin->getObject($id); @@ -1353,6 +1362,7 @@ protected function isPreviewDeclined() */ protected function getAclUsers() { + // NEXT_MAJOR: Remove this code until the commented code and uncomment it; $aclUsers = []; $userManagerServiceName = $this->container->getParameter('sonata.admin.security.acl_user_manager'); @@ -1365,6 +1375,14 @@ protected function getAclUsers() } return \is_array($aclUsers) ? new \ArrayIterator($aclUsers) : $aclUsers; + +// if (!$this->has('sonata.admin.security.acl_user_manager')) { +// return new \ArrayIterator([]); +// } +// +// $aclUsers = $this->get('sonata.admin.security.acl_user_manager')->findUsers(); +// +// return \is_array($aclUsers) ? new \ArrayIterator($aclUsers) : $aclUsers; } /** diff --git a/src/DependencyInjection/Compiler/CheckAclUserManagerCompilerPass.php b/src/DependencyInjection/Compiler/CheckAclUserManagerCompilerPass.php new file mode 100644 index 0000000000..a72556aef0 --- /dev/null +++ b/src/DependencyInjection/Compiler/CheckAclUserManagerCompilerPass.php @@ -0,0 +1,53 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Sonata\AdminBundle\DependencyInjection\Compiler; + +use Sonata\AdminBundle\Util\AdminAclUserManagerInterface; +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\ContainerBuilder; + +/** + * NEXT_MAJOR: Remove this class. + * + * @internal + */ +final class CheckAclUserManagerCompilerPass implements CompilerPassInterface +{ + public function process(ContainerBuilder $container): void + { + if (!$container->hasParameter('sonata.admin.security.acl_user_manager') + || !$container->hasParameter('sonata.admin.security.fos_user_autoconfigured')) { + return; + } + + $userManagerServiceName = $container->getParameter('sonata.admin.security.acl_user_manager'); + + if (null === $userManagerServiceName + || !$container->hasDefinition($userManagerServiceName) + || $container->getParameter('sonata.admin.security.fos_user_autoconfigured')) { + return; + } + + $userManagerDefinition = $container->findDefinition($userManagerServiceName); + + if (!is_a($userManagerDefinition->getClass(), AdminAclUserManagerInterface::class, true)) { + @trigger_error(sprintf( + 'Configuring the service in sonata_admin.security.acl_user_manager without implementing "%s"' + .' is deprecated since sonata-project/admin-bundle 3.x and will throw an "%s" exception in 4.0.', + AdminAclUserManagerInterface::class, + \InvalidArgumentException::class + ), E_USER_DEPRECATED); + } + } +} diff --git a/src/DependencyInjection/SonataAdminExtension.php b/src/DependencyInjection/SonataAdminExtension.php index 7ac231b09e..74df9bea46 100644 --- a/src/DependencyInjection/SonataAdminExtension.php +++ b/src/DependencyInjection/SonataAdminExtension.php @@ -15,6 +15,8 @@ use Sonata\AdminBundle\DependencyInjection\Compiler\ModelManagerCompilerPass; use Sonata\AdminBundle\Model\ModelManagerInterface; +// NEXT_MAJOR: Uncomment this line. +//use Sonata\AdminBundle\Util\AdminAclUserManagerInterface; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface; @@ -115,12 +117,21 @@ public function load(array $configs, ContainerBuilder $container) $container->setParameter('sonata.admin.configuration.default_icon', $config['options']['default_icon']); $container->setParameter('sonata.admin.configuration.breadcrumbs', $config['breadcrumbs']); + // NEXT_MAJOR: Remove this block and uncomment the one below. if (null === $config['security']['acl_user_manager'] && isset($bundles['FOSUserBundle'])) { + $container->setParameter('sonata.admin.security.fos_user_autoconfigured', true); $container->setParameter('sonata.admin.security.acl_user_manager', 'fos_user.user_manager'); } else { + $container->setParameter('sonata.admin.security.fos_user_autoconfigured', false); $container->setParameter('sonata.admin.security.acl_user_manager', $config['security']['acl_user_manager']); } + // NEXT_MAJOR: Uncomment this code. + //if (null !== $config['security']['acl_user_manager']) { + // $container->setAlias('sonata.admin.security.acl_user_manager', $config['security']['acl_user_manager']); + // $container->setAlias(AdminAclUserManagerInterface::class, 'sonata.admin.security.acl_user_manager'); + //} + $container->setAlias('sonata.admin.security.handler', $config['security']['handler']); switch ($config['security']['handler']) { diff --git a/src/Util/AdminAclUserManagerInterface.php b/src/Util/AdminAclUserManagerInterface.php new file mode 100644 index 0000000000..8bc57e3739 --- /dev/null +++ b/src/Util/AdminAclUserManagerInterface.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Sonata\AdminBundle\Util; + +/** + * @author Mathieu Petrini + */ +interface AdminAclUserManagerInterface +{ + /** + * Batch configure the ACLs for all objects handled by an Admin. + */ + public function findUsers(): iterable; +} diff --git a/tests/Controller/CRUDControllerTest.php b/tests/Controller/CRUDControllerTest.php index b53fe3125b..f163d8f7f9 100644 --- a/tests/Controller/CRUDControllerTest.php +++ b/tests/Controller/CRUDControllerTest.php @@ -2662,6 +2662,30 @@ public function testAclActionAclNotEnabled(): void $this->controller->aclAction(null); } + /** + * NEXT_MAJOR: Remove this test. + * + * @group legacy + */ + public function testAclTriggerDeprecationWithoutConfiguringUserManager(): void + { + $this->admin->expects($this->once()) + ->method('isAclEnabled') + ->willReturn(true); + + $this->admin->expects($this->once()) + ->method('getObject') + ->willReturn(false); + + $this->container->setParameter('sonata.admin.security.fos_user_autoconfigured', true); + + $this->expectDeprecation('Not configuring "acl_user_manager" and using ACL security handler is deprecated since sonata-project/admin-bundle 3.x and will not work on 4.x. You MUST specify the service name under "sonata_admin.security.acl_user_manager" option.'); + + $this->expectException(NotFoundHttpException::class); + + $this->controller->aclAction(null); + } + public function testAclActionNotFoundException(): void { $this->expectException(NotFoundHttpException::class); diff --git a/tests/DependencyInjection/Compiler/CheckAclUserManagerCompilerPassTest.php b/tests/DependencyInjection/Compiler/CheckAclUserManagerCompilerPassTest.php new file mode 100644 index 0000000000..df9e4fe6ba --- /dev/null +++ b/tests/DependencyInjection/Compiler/CheckAclUserManagerCompilerPassTest.php @@ -0,0 +1,47 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Sonata\AdminBundle\Tests\DependencyInjection\Compiler; + +use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractCompilerPassTestCase; +use Sonata\AdminBundle\DependencyInjection\Compiler\CheckAclUserManagerCompilerPass; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; + +/** + * NEXT_MAJOR: Remove this test. + * + * @group legacy + */ +final class CheckAclUserManagerCompilerPassTest extends AbstractCompilerPassTestCase +{ + use ExpectDeprecationTrait; + + public function testTriggersADeprecationIfItAclUserManagerIsNotProperlyConfigured(): void + { + $aclUserManager = new Definition(\stdClass::class); + $this->setDefinition('acl_user_manager', $aclUserManager); + $this->setParameter('sonata.admin.security.acl_user_manager', 'acl_user_manager'); + $this->setParameter('sonata.admin.security.fos_user_autoconfigured', false); + + $this->expectDeprecation('Configuring the service in sonata_admin.security.acl_user_manager without implementing "Sonata\AdminBundle\Util\AdminAclUserManagerInterface" is deprecated since sonata-project/admin-bundle 3.x and will throw an "InvalidArgumentException" exception in 4.0.'); + + $this->compile(); + } + + protected function registerCompilerPass(ContainerBuilder $container): void + { + $container->addCompilerPass(new CheckAclUserManagerCompilerPass()); + } +}