-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Original help message:
Proposed help message:
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
This is quite different from what you have said. @wouterj don't you think that the documentation should better explain this strange behavior? |
||
EOF | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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') | ||
; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the point of this method? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need to boot it. the Application will do it There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
||
$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; | ||
} | ||
} |
There was a problem hiding this comment.
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
andCommand::IsEnable
methodThere was a problem hiding this comment.
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 arequire-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 installdoctrine-bundle
before running the tests) and update the doc to explain that tests should be run withdoctrine-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.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're welcome.