diff --git a/core/lib/Drupal/Core/Access/AccessResult.php b/core/lib/Drupal/Core/Access/AccessResult.php index c87b80f..93d804d 100644 --- a/core/lib/Drupal/Core/Access/AccessResult.php +++ b/core/lib/Drupal/Core/Access/AccessResult.php @@ -159,6 +159,23 @@ public static function allowedIfHasPermission(AccountInterface $account, $permis } /** + * Convenience method, creates an AccessResult object and calls allowIfHasPermissions(). + * + * @param \Drupal\Core\Session\AccountInterface $account + * The account for which to check a permission. + * @param array $permissions + * The permissions to check for. + * @param string $conjunction + * (optional) 'AND' if all permissions are required, 'OR' in case just one. + * Defaults to 'AND' + * + * @return \Drupal\Core\Access\AccessResult + */ + public static function allowedIfHasPermissions(AccountInterface $account, $permissions, $conjunction = 'AND') { + return static::create()->allowIfHasPermissions($account, $permissions, $conjunction); + } + + /** * {@inheritdoc} */ public function isAllowed() { @@ -407,6 +424,46 @@ public function allowIfHasPermission(AccountInterface $account, $permission) { } /** + * Convenience method, checks multiple permissions and a conjection. + * + * On top of that it calls ::cachePerRole(). + * + * @param \Drupal\Core\Session\AccountInterface $account + * The account for which to check a permission. + * @param array $permissions + * The permissions to check for. + * @param string $conjunction + * (optional) 'AND' if all permissions are required, 'OR' in case just one. + * Defaults to 'AND' + * + * @return $this + */ + public function allowIfHasPermissions(AccountInterface $account, $permissions, $conjunction = 'AND') { + $access = FALSE; + + if ($conjunction == 'AND' && $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; + } + } + } + + $this->allowIf($access)->cachePerRole(); + return $this; + } + + /** * {@inheritdoc} */ public function orIf(AccessResultInterface $other) { diff --git a/core/modules/user/src/Access/PermissionAccessCheck.php b/core/modules/user/src/Access/PermissionAccessCheck.php index 1421dde..bbe48f5 100644 --- a/core/modules/user/src/Access/PermissionAccessCheck.php +++ b/core/modules/user/src/Access/PermissionAccessCheck.php @@ -33,23 +33,11 @@ public function access(Route $route, AccountInterface $account) { // Separate for matching ANY checks. $split = explode(',', $permission); if (count($split) > 1) { - $access = FALSE; - foreach ($split as $permission) { - if ($account->hasPermission($permission) === TRUE) { - $access = TRUE; - break; - } - } - return AccessResult::allowedIf($access); + return AccessResult::allowedIfHasPermissions($account, $split, 'OR'); } else { - // Separate for matching ALL checks. $split = explode('+', $permission); - $access = TRUE; - foreach ($split as $permission) { - $access &= $account->hasPermission($permission); - } - return AccessResult::allowedIf($access); + return AccessResult::allowedIfHasPermissions($account, $split, 'AND'); } } diff --git a/core/modules/user/tests/src/Unit/PermissionAccessCheckTest.php b/core/modules/user/tests/src/Unit/PermissionAccessCheckTest.php index 09683ec..42348b9 100644 --- a/core/modules/user/tests/src/Unit/PermissionAccessCheckTest.php +++ b/core/modules/user/tests/src/Unit/PermissionAccessCheckTest.php @@ -42,12 +42,12 @@ protected function setUp() { */ public function providerTestAccess() { return [ - [[], AccessResult::allowedIf(FALSE)], - [['_permission' => 'allowed'], AccessResult::allowedIf(TRUE)], - [['_permission' => 'denied'], AccessResult::allowedIf(FALSE)], - [['_permission' => 'allowed,denied'], AccessResult::allowedIf(TRUE)], - [['_permission' => 'allowed,denied,other'], AccessResult::allowedIf(TRUE)], - [['_permission' => 'allowed+denied'], AccessResult::allowedIf(FALSE)], + [[], AccessResult::allowedIf(FALSE)->cachePerRole()], + [['_permission' => 'allowed'], AccessResult::allowedIf(TRUE)->cachePerRole()], + [['_permission' => 'denied'], AccessResult::allowedIf(FALSE)->cachePerRole()], + [['_permission' => 'allowed,denied'], AccessResult::allowedIf(TRUE)->cachePerRole()], + [['_permission' => 'allowed,denied,other'], AccessResult::allowedIf(TRUE)->cachePerRole()], + [['_permission' => 'allowed+denied'], AccessResult::allowedIf(FALSE)->cachePerRole()], ]; } diff --git a/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php b/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php index 67d25d5..fea591b 100644 --- a/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php +++ b/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php @@ -529,4 +529,43 @@ public function testCacheabilityMerging() { $this->assertFalse(method_exists($access, 'isCacheable')); } + /** + * Tests allowIfHasPermission(). + * + * @covers ::allowIfHasPermissions + * + * @dataProvider providerTestAllowIfHasPermissions + */ + 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::create()->allowIfHasPermissions($account, $permissions, $conjunction); + $this->assertEquals($expected_access, $access_result); + } + + /** + * Provides data for the testAllowIfHasPermissions method. + * + * @return array + */ + public function providerTestAllowIfHasPermissions() { + 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()], + ]; + } + }