From 9f4e89ed1b69ceb54d253b6510e9a8acc43a43e0 Mon Sep 17 00:00:00 2001
From: Lee Rowlands <lee.rowlands@previousnext.com.au>
Date: Mon, 30 Oct 2023 07:46:42 +1000
Subject: [PATCH] Issue #3366719 by smustgrave, acbramley, yash.rode: Allow
reverting block content with "administer block content" permission
---
.../src/BlockContentAccessControlHandler.php | 73 ++++++++-----------
.../Kernel/BlockContentAccessHandlerTest.php | 18 ++---
2 files changed, 41 insertions(+), 50 deletions(-)
diff --git a/core/modules/block_content/src/BlockContentAccessControlHandler.php b/core/modules/block_content/src/BlockContentAccessControlHandler.php
index 54052feeff8e..8821080a9ce8 100644
--- a/core/modules/block_content/src/BlockContentAccessControlHandler.php
+++ b/core/modules/block_content/src/BlockContentAccessControlHandler.php
@@ -58,49 +58,40 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter
assert($entity instanceof BlockContentInterface);
$bundle = $entity->bundle();
$forbidIfNotReusable = fn (): AccessResultInterface => AccessResult::forbiddenIf($entity->isReusable() === FALSE, sprintf('Block content must be reusable to use `%s` operation', $operation));
- $access = match ($operation) {
- // Allow view and update access to user with the 'edit any (type) block
- // content' permission or the 'administer blocks' permission.
- 'view' => AccessResult::allowedIf($entity->isPublished())
- ->orIf(AccessResult::allowedIfHasPermissions($account, [
+ $access = AccessResult::allowedIfHasPermissions($account, ['administer block content']);
+ if (!$access->isAllowed()) {
+ $access = match ($operation) {
+ // Allow view and update access to user with the 'edit any (type) block
+ // content' permission or the 'administer block content' permission.
+ 'view' => AccessResult::allowedIf($entity->isPublished())
+ ->orIf(AccessResult::allowedIfHasPermissions($account, [
+ 'access block library',
+ ])),
+ 'update' => AccessResult::allowedIfHasPermissions($account, [
'access block library',
- ]))->orIf(AccessResult::allowedIfHasPermissions($account, [
- 'administer block content',
- ])),
- 'update' => AccessResult::allowedIfHasPermissions($account, [
- 'access block library',
- 'edit any ' . $bundle . ' block content',
- ])->orIf(AccessResult::allowedIfHasPermissions($account, [
- 'administer block content',
- ])),
- 'delete' => AccessResult::allowedIfHasPermissions($account, [
- 'access block library',
- 'delete any ' . $bundle . ' block content',
- ])->orIf(AccessResult::allowedIfHasPermissions($account, [
- 'administer block content',
- ])),
- // Revisions.
- 'view all revisions' => AccessResult::allowedIfHasPermissions($account, [
- 'access block library',
- 'view any ' . $bundle . ' block content history',
- ])->orIf(AccessResult::allowedIfHasPermissions($account, [
- 'administer block content',
- ])),
- 'revert' => AccessResult::allowedIfHasPermissions($account, [
- 'access block library',
- 'revert any ' . $bundle . ' block content revisions',
- ])->orIf($forbidIfNotReusable()),
- 'delete revision' => AccessResult::allowedIfHasPermissions($account, [
- 'access block library',
- 'delete any ' . $bundle . ' block content revisions',
- ])
- ->orIf($forbidIfNotReusable())
- ->orIf(AccessResult::allowedIfHasPermissions($account, [
- 'administer block content',
- ])),
+ 'edit any ' . $bundle . ' block content',
+ ]),
+ 'delete' => AccessResult::allowedIfHasPermissions($account, [
+ 'access block library',
+ 'delete any ' . $bundle . ' block content',
+ ]),
+ // Revisions.
+ 'view all revisions' => AccessResult::allowedIfHasPermissions($account, [
+ 'access block library',
+ 'view any ' . $bundle . ' block content history',
+ ]),
+ 'revert' => AccessResult::allowedIfHasPermissions($account, [
+ 'access block library',
+ 'revert any ' . $bundle . ' block content revisions',
+ ])->orIf($forbidIfNotReusable()),
+ 'delete revision' => AccessResult::allowedIfHasPermissions($account, [
+ 'access block library',
+ 'delete any ' . $bundle . ' block content revisions',
+ ])->orIf($forbidIfNotReusable()),
- default => parent::checkAccess($entity, $operation, $account),
- };
+ default => parent::checkAccess($entity, $operation, $account),
+ };
+ }
// Add the entity as a cacheable dependency because access will at least be
// determined by whether the block is reusable.
diff --git a/core/modules/block_content/tests/src/Kernel/BlockContentAccessHandlerTest.php b/core/modules/block_content/tests/src/Kernel/BlockContentAccessHandlerTest.php
index b4929ce54787..cae7c2215424 100644
--- a/core/modules/block_content/tests/src/Kernel/BlockContentAccessHandlerTest.php
+++ b/core/modules/block_content/tests/src/Kernel/BlockContentAccessHandlerTest.php
@@ -447,7 +447,7 @@ public function providerTestAccess(): array {
NULL,
AccessResultNeutral::class,
];
- $cases['view all revisions:administer blocks'] = [
+ $cases['view all revisions:administer block content'] = [
'view all revisions',
TRUE,
TRUE,
@@ -485,16 +485,16 @@ public function providerTestAccess(): array {
NULL,
AccessResultNeutral::class,
];
- $cases['revert:administer blocks:latest'] = [
+ $cases['revert:administer block content:latest'] = [
'revert',
TRUE,
TRUE,
- ['access block library'],
+ ['administer block content'],
TRUE,
NULL,
AccessResultForbidden::class,
];
- $cases['revert:administer blocks:historical'] = [
+ $cases['revert:administer block content:historical'] = [
'revert',
TRUE,
TRUE,
@@ -507,7 +507,7 @@ public function providerTestAccess(): array {
'revert',
TRUE,
TRUE,
- ['administer blocks'],
+ ['administer block content'],
TRUE,
NULL,
AccessResultForbidden::class,
@@ -551,16 +551,16 @@ public function providerTestAccess(): array {
NULL,
AccessResultNeutral::class,
];
- $cases['delete revision:administer blocks:latest'] = [
+ $cases['delete revision:administer block content:latest'] = [
'delete revision',
TRUE,
TRUE,
- ['administer blocks'],
+ ['administer block content'],
TRUE,
NULL,
AccessResultForbidden::class,
];
- $cases['delete revision:administer blocks:historical'] = [
+ $cases['delete revision:administer block content:historical'] = [
'delete revision',
TRUE,
TRUE,
@@ -573,7 +573,7 @@ public function providerTestAccess(): array {
'delete revision',
TRUE,
TRUE,
- ['administer blocks'],
+ ['administer block content'],
TRUE,
NULL,
AccessResultForbidden::class,
--
GitLab