Commit 8a150288 authored by webchick's avatar webchick

Issue #1986640 by Wim Leers, chx, dawehner, effulgentsia: Support AND/OR...

Issue #1986640 by Wim Leers, chx, dawehner, effulgentsia: Support AND/OR conjunctions for permission checks.
parent 9801d9a5
......@@ -139,13 +139,54 @@ public static function forbiddenIf($condition) {
* The permission to check for.
*
* @return \Drupal\Core\Access\AccessResult
* If the account has the permission, isAlowed() will be TRUE, otherwise
* If the account has the permission, isAllowed() will be TRUE, otherwise
* isNeutral() will be TRUE.
*/
public static function allowedIfHasPermission(AccountInterface $account, $permission) {
return static::allowedIf($account->hasPermission($permission))->cachePerRole();
}
/**
* Creates an allowed access result if the permissions are present, neutral otherwise.
*
* Convenience method, checks the permissions and calls ::cachePerRole().
*
* @param \Drupal\Core\Session\AccountInterface $account
* The account for which to check permissions.
* @param array $permissions
* The permissions to check.
* @param string $conjunction
* (optional) 'AND' if all permissions are required, 'OR' in case just one.
* Defaults to 'AND'
*
* @return \Drupal\Core\Access\AccessResult
* If the account has the permissions, isAllowed() will be TRUE, otherwise
* isNeutral() will be TRUE.
*/
public static function allowedIfHasPermissions(AccountInterface $account, array $permissions, $conjunction = 'AND') {
$access = FALSE;
if ($conjunction == 'AND' && !empty($permissions)) {
$access = TRUE;
foreach ($permissions as $permission) {
if (!$permission_access = $account->hasPermission($permission)) {
$access = FALSE;
break;
}
}
}
else {
foreach ($permissions as $permission) {
if ($permission_access = $account->hasPermission($permission)) {
$access = TRUE;
break;
}
}
}
return static::allowedIf($access)->cachePerRole();
}
/**
* {@inheritdoc}
*
......
......@@ -23,7 +23,7 @@ public function defaultAccess($operation = 'view', AccountInterface $account = N
if ($operation == 'view') {
return AccessResult::allowed();
}
return AccessResult::allowedIf($account->hasPermission('create url aliases') || $account->hasPermission('administer url aliases'))->cachePerRole();
return AccessResult::allowedIfHasPermissions($account, ['create url aliases', 'administer url aliases'], 'OR')->cachePerRole();
}
}
......@@ -46,8 +46,7 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A
* {@inheritdoc}
*/
protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) {
$condition = $account->hasPermission('administer shortcuts') || ($account->hasPermission('access shortcuts') && $account->hasPermission('customize shortcut links'));
return AccessResult::allowedIf($condition)->cachePerRole();
return AccessResult::allowedIfHasPermission($account, 'administer shortcuts')->orIf(AccessResult::allowedIfHasPermissions($account, ['access shortcuts', 'customize shortcut links'], 'AND'));
}
}
......@@ -29,11 +29,11 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A
break;
case 'update':
return AccessResult::allowedIf($account->hasPermission("edit terms in {$entity->bundle()}") || $account->hasPermission('administer taxonomy'))->cachePerRole();
return AccessResult::allowedIfHasPermissions($account, ["edit terms in {$entity->bundle()}", 'administer taxonomy'], 'OR');
break;
case 'delete':
return AccessResult::allowedIf($account->hasPermission("delete terms in {$entity->bundle()}") || $account->hasPermission('administer taxonomy'))->cachePerRole();
return AccessResult::allowedIfHasPermissions($account, ["delete terms in {$entity->bundle()}", 'administer taxonomy'], 'OR');
break;
default:
......
......@@ -31,6 +31,15 @@ class PermissionAccessCheck implements AccessInterface {
*/
public function access(Route $route, AccountInterface $account) {
$permission = $route->getRequirement('_permission');
return AccessResult::allowedIfHasPermission($account, $permission);
// Allow to conjunct the permissions with OR ('+') or AND (',').
$split = explode(',', $permission);
if (count($split) > 1) {
return AccessResult::allowedIfHasPermissions($account, $split, 'AND');
}
else {
$split = explode('+', $permission);
return AccessResult::allowedIfHasPermissions($account, $split, 'OR');
}
}
}
<?php
/**
* @file
* Contains \Drupal\Tests\user\Unit\PermissionAccessCheckTest.
*/
namespace Drupal\Tests\user\Unit;
use Drupal\Core\Access\AccessResult;
use Drupal\Tests\UnitTestCase;
use Drupal\user\Access\PermissionAccessCheck;
use Symfony\Component\Routing\Route;
/**
* @coversDefaultClass \Drupal\user\Access\PermissionAccessCheck
* @group Routing
* @group AccessF
*/
class PermissionAccessCheckTest extends UnitTestCase {
/**
* The tested access checker.
*
* @var \Drupal\user\Access\PermissionAccessCheck
*/
public $accessCheck;
/**
* {@inheritdoc}
*/
protected function setUp() {
parent::setUp();
$this->accessCheck = new PermissionAccessCheck();
}
/**
* Provides data for the testAccess method.
*
* @return array
*/
public function providerTestAccess() {
$allowed = AccessResult::allowedIf(TRUE)->cachePerRole();
$neutral = AccessResult::allowedIf(FALSE)->cachePerRole();
return [
[[], $neutral],
[['_permission' => 'allowed'], $allowed],
[['_permission' => 'denied'], $neutral],
[['_permission' => 'allowed+denied'], $allowed],
[['_permission' => 'allowed+denied+other'], $allowed],
[['_permission' => 'allowed-denied'], $neutral],
];
}
/**
* Tests the access check method.
*
* @dataProvider providerTestAccess
* @covers ::access
*/
public function testAccess($requirements, $access) {
$user = $this->getMock('Drupal\Core\Session\AccountInterface');
$user->expects($this->any())
->method('hasPermission')
->will($this->returnValueMap([
['allowed', TRUE],
['denied', FALSE],
['other', FALSE]
]
));
$route = new Route('', [], $requirements);
$this->assertEquals($access, $this->accessCheck->access($route, $user));
}
}
......@@ -32,7 +32,7 @@ protected function assertDefaultCacheability(AccessResult $access) {
* Tests the construction of an AccessResult object.
*
* @covers ::__construct
* @covers ::create
* @covers ::neutral
* @covers ::getCacheBin
*/
public function testConstruction() {
......@@ -55,7 +55,6 @@ public function testConstruction() {
}
/**
* @covers ::allow
* @covers ::allowed
* @covers ::isAllowed
* @covers ::isForbidden
......@@ -75,7 +74,6 @@ public function testAccessAllowed() {
}
/**
* @covers ::forbid
* @covers ::forbidden
* @covers ::isAllowed
* @covers ::isForbidden
......@@ -95,7 +93,6 @@ public function testAccessForbidden() {
}
/**
* @covers ::allowIf
* @covers ::allowedIf
* @covers ::isAllowed
* @covers ::isForbidden
......@@ -116,7 +113,6 @@ public function testAccessConditionallyAllowed() {
}
/**
* @covers ::forbidIf
* @covers ::forbiddenIf
* @covers ::isAllowed
* @covers ::isForbidden
......@@ -325,7 +321,6 @@ public function testCacheMaxAge() {
* @covers ::getCacheKeys
* @covers ::cachePerRole
* @covers ::cachePerUser
* @covers ::allowIfHasPermission
* @covers ::allowedIfHasPermission
*/
public function testCacheContexts() {
......@@ -807,6 +802,45 @@ public function testAndOrCacheabilityPropagation(AccessResultInterface $first, $
}
}
/**
* Tests allowedIfHasPermissions().
*
* @covers ::allowedIfHasPermissions
*
* @dataProvider providerTestAllowedIfHasPermissions
*/
public function testAllowIfHasPermissions($permissions, $conjunction, $expected_access) {
$account = $this->getMock('\Drupal\Core\Session\AccountInterface');
$account->expects($this->any())
->method('hasPermission')
->willReturnMap([
['allowed', TRUE],
['denied', FALSE],
]);
$access_result = AccessResult::allowedIfHasPermissions($account, $permissions, $conjunction);
$this->assertEquals($expected_access, $access_result);
}
/**
* Provides data for the testAllowedIfHasPermissions() method.
*
* @return array
*/
public function providerTestAllowedIfHasPermissions() {
return [
[[], 'AND', AccessResult::allowedIf(FALSE)->cachePerRole()],
[[], 'OR', AccessResult::allowedIf(FALSE)->cachePerRole()],
[['allowed'], 'OR', AccessResult::allowedIf(TRUE)->cachePerRole()],
[['allowed'], 'AND', AccessResult::allowedIf(TRUE)->cachePerRole()],
[['denied'], 'OR', AccessResult::allowedIf(FALSE)->cachePerRole()],
[['denied'], 'AND', AccessResult::allowedIf(FALSE)->cachePerRole()],
[['allowed', 'denied'], 'OR', AccessResult::allowedIf(TRUE)->cachePerRole()],
[['allowed', 'denied', 'other'], 'OR', AccessResult::allowedIf(TRUE)->cachePerRole()],
[['allowed', 'denied'], 'AND', AccessResult::allowedIf(FALSE)->cachePerRole()],
];
}
}
class UncacheableTestAccessResult implements AccessResultInterface {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment