Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate acl_user_manager without implementing AdminAclUserManagerInterface #6480

Merged
merged 1 commit into from
Oct 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions UPGRADE-3.x.md
Original file line number Diff line number Diff line change
@@ -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
<?php

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();
}
}
```

UPGRADE FROM 3.76 to 3.77
=========================

Expand Down
69 changes: 46 additions & 23 deletions docs/reference/security.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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::
Expand Down Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions src/Controller/CRUDController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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');
Expand All @@ -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;
VincentLanglet marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

declare(strict_types=1);

/*
* This file is part of the Sonata Project package.
*
* (c) Thomas Rabaix <[email protected]>
*
* 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)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a warning: some time ago I added a similar check in our packages (I don't remember which one), and I had to add an extra DI resolution about the return value of Definition::getClass(), since it can be also a parameter (by instance %admin.acl.user_manager.class%) which must be resolved first in order to know the real class.

@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);
}
}
}
11 changes: 11 additions & 0 deletions src/DependencyInjection/SonataAdminExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

take a look at my PR to see the correct alias, otherwise it does not work (at least on my tests)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also left this one because from the controller we are fetching it with get('sonata.admin.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']) {
Expand Down
25 changes: 25 additions & 0 deletions src/Util/AdminAclUserManagerInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

declare(strict_types=1);

/*
* This file is part of the Sonata Project package.
*
* (c) Thomas Rabaix <[email protected]>
*
* 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 <[email protected]>
*/
interface AdminAclUserManagerInterface
{
/**
* Batch configure the ACLs for all objects handled by an Admin.
*/
public function findUsers(): iterable;
}
24 changes: 24 additions & 0 deletions tests/Controller/CRUDControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2662,6 +2662,30 @@ public function testAclActionAclNotEnabled(): void
$this->controller->aclAction(null);
}

/**
* NEXT_MAJOR: Remove this test.
*
* @group legacy
VincentLanglet marked this conversation as resolved.
Show resolved Hide resolved
*/
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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

declare(strict_types=1);

/*
* This file is part of the Sonata Project package.
*
* (c) Thomas Rabaix <[email protected]>
*
* 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());
}
}