Skip to content

[SecurityBundle] added acl:set command #9990

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

Merged
merged 1 commit into from
Jun 3, 2014
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
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
"doctrine/data-fixtures": "1.0.*",
"doctrine/dbal": "~2.2",
"doctrine/orm": "~2.2,>=2.2.3",
"doctrine/doctrine-bundle": "~1.2",
Copy link
Member

Choose a reason for hiding this comment

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

A soft dependency could be better.

You can play with class_exist and Command::IsEnable method

Copy link
Member Author

Choose a reason for hiding this comment

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

@lyrixx doctrine-bundle is only needed to run the tests. It's a require-dev dependency.
Making it a soft dependency is awkward because all functional tests on the ACL system will be skipped by default.
If someone clone the repository, start hacking, run phpunit and make a PR, he can silently broke the ACL system.

It will also require tweaking .travis.yml (to make Travis install doctrine-bundle before running the tests) and update the doc to explain that tests should be run with doctrine-bundle installed.

If it's really annoying to have doctrine-bundle as a development dependency, I prefer registering the DBAL connection directly in the test app's kernel.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me know which solution is better.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry. I did not see it was a dev deps ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

You're welcome.

"monolog/monolog": "~1.3",
"propel/propel1": "1.6.*",
"ircmaxell/password-compat": "1.0.*",
Expand Down
171 changes: 171 additions & 0 deletions src/Symfony/Bundle/SecurityBundle/Command/SetAclCommand.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\SecurityBundle\Command;

use Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Security\Acl\Domain\ObjectIdentity;
use Symfony\Component\Security\Acl\Domain\RoleSecurityIdentity;
use Symfony\Component\Security\Acl\Domain\UserSecurityIdentity;
use Symfony\Component\Security\Acl\Exception\AclAlreadyExistsException;
use Symfony\Component\Security\Acl\Permission\MaskBuilder;
use Symfony\Component\Security\Acl\Model\MutableAclProviderInterface;

/**
* Sets ACL for objects
*
* @author Kévin Dunglas <kevin@les-tilleuls.coop>
*/
class SetAclCommand extends ContainerAwareCommand
{
/**
* {@inheritdoc}
*/
public function isEnabled()
{
if (!$this->getContainer()->has('security.acl.provider')) {
return false;
}

$provider = $this->getContainer()->get('security.acl.provider');
if (!$provider instanceof MutableAclProviderInterface) {
return false;
}

return parent::isEnabled();
}

/**
* {@inheritdoc}
*/
protected function configure()
{
$this
->setName('acl:set')
->setDescription('Sets ACL for objects')
->setHelp(<<<EOF
The <info>%command.name%</info> command sets ACL.
The ACL system must have been initialized with the <info>init:acl</info> command.

To set <comment>VIEW</comment> and <comment>EDIT</comment> permissions for the user <comment>kevin</comment> on the instance of <comment>Acme\MyClass</comment> having the identifier <comment>42</comment>:

<info>php %command.full_name% --user=Symfony/Component/Security/Core/User/User:kevin VIEW EDIT Acme/MyClass:42</info>

Note that you can use <comment>/</comment> instead of <comment>\\ </comment>for the namespace delimiter to avoid any
problem.

To set permissions for a role, use the <info>--role</info> option:

<info>php %command.full_name% --role=ROLE_USER VIEW Acme/MyClass:1936</info>

To set permissions at the class scope, use the <info>--class-scope</info> option:

<info>php %command.full_name% --class-scope --user=Symfony/Component/Security/Core/User/User:anne OWNER Acme/MyClass:42</info>
Copy link
Member

Choose a reason for hiding this comment

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

Hi @dunglas, I'm redacting a "New in Symfony 2.6" blog post about the new acl:set and I wonder if this help message is correct. If you are using the --class-scope, you are granting access to all the objects of the same class, so the trailing object id (42 in this case) should be removed, right?

Original help message:

<info>php %command.full_name% --class-scope --user=Symfony/Component/Security/Core/User/User:anne OWNER Acme/MyClass:42</info>

Proposed help message:

<info>php %command.full_name% --class-scope --user=Symfony/Component/Security/Core/User/User:anne OWNER Acme/MyClass</info>

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @javiereguiluz. This help message is correct. It should not be modified. This how the underlying ACL system works (and I agree that this is far from perfect).

The class scope can be set only is you precise an object identity (otherwise an error will be thrown). Technically, it will not grant access to all objects of the same class but only to all objects of this class present in the ACL table.

e.g:

Copy link
Member

Choose a reason for hiding this comment

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

@dunglas thanks for your reply. I now understand that everything is correct. However, I find this behavior amazingly counterintuitive. Moreover, in the scopes for access control entries section of the documentation it says:

Class-Scope: These entries apply to all objects with the same class.

This is quite different from what you have said. @wouterj don't you think that the documentation should better explain this strange behavior?

EOF
Copy link
Member

Choose a reason for hiding this comment

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

The Help should be expanded to explain the main use cases (probably using the defined options).

)
->addArgument('arguments', InputArgument::IS_ARRAY | InputArgument::REQUIRED, 'A list of permissions and object identities (class name and ID separated by a column)')
->addOption('user', null, InputOption::VALUE_REQUIRED, 'A list of security identities')
->addOption('role', null, InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, 'A list of roles')
->addOption('class-scope', null, InputOption::VALUE_NONE, 'Use class-scope entries')
;
}
Copy link
Member

Choose a reason for hiding this comment

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

I would remove all shortcuts here as they don't really make sense to me (also, a shortcut should be one letter only).


/**
* {@inheritdoc}
*/
protected function execute(InputInterface $input, OutputInterface $output)
{
// Parse arguments
$objectIdentities = array();
$maskBuilder = $this->getMaskBuilder();
foreach ($input->getArgument('arguments') as $argument) {
$data = explode(':', $argument, 2);

if (count($data) > 1) {
$objectIdentities[] = new ObjectIdentity($data[1], strtr($data[0], '/', '\\'));
} else {
$maskBuilder->add($data[0]);
}
}

// Build permissions mask
$mask = $maskBuilder->get();

$userOption = $input->getOption('user');
$roleOption = $input->getOption('role');
$classScopeOption = $input->getOption('class-scope');

if (empty($userOption) && empty($roleOption)) {
throw new \InvalidArgumentException('A Role or a User must be specified.');
}

// Create security identities
$securityIdentities = array();

if ($userOption) {
foreach ($userOption as $user) {
$data = explode(':', $user, 2);

if (count($data) === 1) {
throw new \InvalidArgumentException('The user must follow the format "Acme/MyUser:username".');
}

$securityIdentities[] = new UserSecurityIdentity($data[1], strtr($data[0], '/', '\\'));
}
}

if ($roleOption) {
foreach ($roleOption as $role) {
$securityIdentities[] = new RoleSecurityIdentity($role);
}
}

/** @var $container \Symfony\Component\DependencyInjection\ContainerInterface */
$container = $this->getContainer();
/** @var $aclProvider MutableAclProviderInterface */
$aclProvider = $container->get('security.acl.provider');

// Sets ACL
foreach ($objectIdentities as $objectIdentity) {
// Creates a new ACL if it does not already exist
try {
$aclProvider->createAcl($objectIdentity);
} catch (AclAlreadyExistsException $e) {
}

$acl = $aclProvider->findAcl($objectIdentity, $securityIdentities);

foreach ($securityIdentities as $securityIdentity) {
if ($classScopeOption) {
$acl->insertClassAce($securityIdentity, $mask);
} else {
$acl->insertObjectAce($securityIdentity, $mask);
}
}

$aclProvider->updateAcl($acl);
}
}

/**
* Gets the mask builder
*
* @return MaskBuilder
*/
protected function getMaskBuilder()
Copy link
Member

Choose a reason for hiding this comment

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

What's the point of this method?

Copy link
Member Author

Choose a reason for hiding this comment

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

@fabpot The instantiation of the MaskBuilder object is done in a separate method to be easily overridable to use a custom one (e.g. the SonataAdmin one).

{
return new MaskBuilder();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\AclBundle;

use Symfony\Component\HttpKernel\Bundle\Bundle;

/**
* @author Kévin Dunglas <kevin@les-tilleuls.coop>
*/
class AclBundle extends Bundle
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\AclBundle\Entity;

/**
* Car
*
* @author Kévin Dunglas <kevin@les-tilleuls.coop>
*/
class Car
{
public $id;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
<?php

namespace Symfony\Bundle\SecurityBundle\Tests\Functional;

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
use Symfony\Bundle\FrameworkBundle\Console\Application;
use Symfony\Bundle\SecurityBundle\Command\InitAclCommand;
use Symfony\Bundle\SecurityBundle\Command\SetAclCommand;
use Symfony\Component\Console\Tester\CommandTester;
use Symfony\Component\Security\Acl\Domain\ObjectIdentity;
use Symfony\Component\Security\Acl\Domain\RoleSecurityIdentity;
use Symfony\Component\Security\Acl\Domain\UserSecurityIdentity;
use Symfony\Component\Security\Acl\Exception\NoAceFoundException;
use Symfony\Component\Security\Acl\Permission\BasicPermissionMap;

/**
* Tests SetAclCommand
*
* @author Kévin Dunglas <kevin@les-tilleuls.coop>
*/
class SetAclCommandTest extends WebTestCase
{
const OBJECT_CLASS = 'Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\AclBundle\Entity\Car';
const SECURITY_CLASS = 'Symfony\Component\Security\Core\User\User';

public function testSetAclUser()
{
$objectId = 1;
$securityUsername1 = 'kevin';
$securityUsername2 = 'anne';
$grantedPermission1 = 'VIEW';
$grantedPermission2 = 'EDIT';

$application = $this->getApplication();
$application->add(new SetAclCommand());

$setAclCommand = $application->find('acl:set');
$setAclCommandTester = new CommandTester($setAclCommand);
$setAclCommandTester->execute(array(
'command' => 'acl:set',
'arguments' => array($grantedPermission1, $grantedPermission2, sprintf('%s:%s', self::OBJECT_CLASS, $objectId)),
'--user' => array(sprintf('%s:%s', self::SECURITY_CLASS, $securityUsername1), sprintf('%s:%s', self::SECURITY_CLASS, $securityUsername2))
));

$objectIdentity = new ObjectIdentity($objectId, self::OBJECT_CLASS);
$securityIdentity1 = new UserSecurityIdentity($securityUsername1, self::SECURITY_CLASS);
$securityIdentity2 = new UserSecurityIdentity($securityUsername2, self::SECURITY_CLASS);
$permissionMap = new BasicPermissionMap();

/** @var \Symfony\Component\Security\Acl\Model\AclProviderInterface $aclProvider */
$aclProvider = $application->getKernel()->getContainer()->get('security.acl.provider');
$acl = $aclProvider->findAcl($objectIdentity, array($securityIdentity1));

$this->assertTrue($acl->isGranted($permissionMap->getMasks($grantedPermission1, null), array($securityIdentity1)));
$this->assertTrue($acl->isGranted($permissionMap->getMasks($grantedPermission1, null), array($securityIdentity2)));
$this->assertTrue($acl->isGranted($permissionMap->getMasks($grantedPermission2, null), array($securityIdentity2)));

try {
$acl->isGranted($permissionMap->getMasks('OWNER', null), array($securityIdentity1));
$this->fail('NoAceFoundException not throwed');
} catch (NoAceFoundException $e) {
}

try {
$acl->isGranted($permissionMap->getMasks('OPERATOR', null), array($securityIdentity2));
$this->fail('NoAceFoundException not throwed');
} catch (NoAceFoundException $e) {
}
}

public function testSetAclRole()
{
$objectId = 1;
$securityUsername = 'kevin';
$grantedPermission = 'VIEW';
$role = 'ROLE_ADMIN';

$application = $this->getApplication();
$application->add(new SetAclCommand());

$setAclCommand = $application->find('acl:set');
$setAclCommandTester = new CommandTester($setAclCommand);
$setAclCommandTester->execute(array(
'command' => 'acl:set',
'arguments' => array($grantedPermission, sprintf('%s:%s', strtr(self::OBJECT_CLASS, '\\', '/'), $objectId)),
'--role' => array($role)
));

$objectIdentity = new ObjectIdentity($objectId, self::OBJECT_CLASS);
$userSecurityIdentity = new UserSecurityIdentity($securityUsername, self::SECURITY_CLASS);
$roleSecurityIdentity = new RoleSecurityIdentity($role);
$permissionMap = new BasicPermissionMap();

/** @var \Symfony\Component\Security\Acl\Model\AclProviderInterface $aclProvider */
$aclProvider = $application->getKernel()->getContainer()->get('security.acl.provider');
$acl = $aclProvider->findAcl($objectIdentity, array($roleSecurityIdentity, $userSecurityIdentity));

$this->assertTrue($acl->isGranted($permissionMap->getMasks($grantedPermission, null), array($roleSecurityIdentity)));
$this->assertTrue($acl->isGranted($permissionMap->getMasks($grantedPermission, null), array($roleSecurityIdentity)));

try {
$acl->isGranted($permissionMap->getMasks('VIEW', null), array($userSecurityIdentity));
$this->fail('NoAceFoundException not throwed');
} catch (NoAceFoundException $e) {
}

try {
$acl->isGranted($permissionMap->getMasks('OPERATOR', null), array($userSecurityIdentity));
$this->fail('NoAceFoundException not throwed');
} catch (NoAceFoundException $e) {
}
}

public function testSetAclClassScope()
{
$objectId = 1;
$grantedPermission = 'VIEW';
$role = 'ROLE_USER';

$application = $this->getApplication();
$application->add(new SetAclCommand());

$setAclCommand = $application->find('acl:set');
$setAclCommandTester = new CommandTester($setAclCommand);
$setAclCommandTester->execute(array(
'command' => 'acl:set',
'arguments' => array($grantedPermission, sprintf('%s:%s', self::OBJECT_CLASS, $objectId)),
'--class-scope' => true,
'--role' => array($role)
));

$objectIdentity1 = new ObjectIdentity($objectId, self::OBJECT_CLASS);
$objectIdentity2 = new ObjectIdentity(2, self::OBJECT_CLASS);
$roleSecurityIdentity = new RoleSecurityIdentity($role);
$permissionMap = new BasicPermissionMap();

/** @var \Symfony\Component\Security\Acl\Model\AclProviderInterface $aclProvider */
$aclProvider = $application->getKernel()->getContainer()->get('security.acl.provider');

$acl1 = $aclProvider->findAcl($objectIdentity1, array($roleSecurityIdentity));
$this->assertTrue($acl1->isGranted($permissionMap->getMasks($grantedPermission, null), array($roleSecurityIdentity)));

$acl2 = $aclProvider->createAcl($objectIdentity2);
$this->assertTrue($acl2->isGranted($permissionMap->getMasks($grantedPermission, null), array($roleSecurityIdentity)));
}

private function getApplication()
{
$kernel = $this->createKernel(array('test_case' => 'Acl'));
$kernel->boot();
Copy link
Member

Choose a reason for hiding this comment

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

no need to boot it. the Application will do it

Copy link
Member Author

Choose a reason for hiding this comment

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

@stof this is necessary to boot the kernel earlier here to run the init:acl command.


$application = new Application($kernel);
$application->add(new InitAclCommand());

$initAclCommand = $application->find('init:acl');
$initAclCommandTester = new CommandTester($initAclCommand);
$initAclCommandTester->execute(array('command' => 'init:acl'));

return $application;
}
}
Loading