diff --git a/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php index ca7c7c8f92bf7edbc7f22a6565242f9aff94226f..aa7460dc31caa44cc5eb0dc29e003d30fc68f60b 100644 --- a/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php +++ b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php @@ -76,6 +76,10 @@ public function access(EntityInterface $entity, $operation, AccountInterface $ac if ($entity instanceof RevisionableInterface) { /** @var \Drupal\Core\Entity\RevisionableInterface $entity */ $cid .= ':' . $entity->getRevisionId(); + // It is not possible to delete or revert the default revision. + if ($entity->isDefaultRevision() && ($operation === 'revert' || $operation === 'delete revision')) { + return $return_as_object ? AccessResult::forbidden() : FALSE; + } } if (($return = $this->getCache($cid, $operation, $langcode, $account)) !== NULL) { diff --git a/core/modules/block_content/src/BlockContentAccessControlHandler.php b/core/modules/block_content/src/BlockContentAccessControlHandler.php index f765517908422eb0ad84f78c7c94ed1ff7137974..54052feeff8e1eac10f8d32857cf2d4b27d20234 100644 --- a/core/modules/block_content/src/BlockContentAccessControlHandler.php +++ b/core/modules/block_content/src/BlockContentAccessControlHandler.php @@ -6,9 +6,9 @@ use Drupal\block_content\Event\BlockContentGetDependencyEvent; use Drupal\Core\Access\AccessResult; use Drupal\Core\Access\AccessResultInterface; +use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityHandlerInterface; use Drupal\Core\Entity\EntityInterface; -use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -57,7 +57,6 @@ public static function createInstance(ContainerInterface $container, EntityTypeI protected function checkAccess(EntityInterface $entity, $operation, AccountInterface $account) { assert($entity instanceof BlockContentInterface); $bundle = $entity->bundle(); - $forbidIfNotDefaultAndLatest = fn (): AccessResultInterface => AccessResult::forbiddenIf($entity->isDefaultRevision() && $entity->isLatestRevision()); $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 @@ -90,12 +89,11 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter 'revert' => AccessResult::allowedIfHasPermissions($account, [ 'access block library', 'revert any ' . $bundle . ' block content revisions', - ])->orIf($forbidIfNotDefaultAndLatest())->orIf($forbidIfNotReusable()), + ])->orIf($forbidIfNotReusable()), 'delete revision' => AccessResult::allowedIfHasPermissions($account, [ 'access block library', 'delete any ' . $bundle . ' block content revisions', ]) - ->orIf($forbidIfNotDefaultAndLatest()) ->orIf($forbidIfNotReusable()) ->orIf(AccessResult::allowedIfHasPermissions($account, [ 'administer block content', diff --git a/core/modules/block_content/tests/src/Functional/BlockContentRevisionDeleteTest.php b/core/modules/block_content/tests/src/Functional/BlockContentRevisionDeleteTest.php index 25fe2f5ad989118a86faaf3ef70a46a886222251..6dac5e751e82e2fc30f02b53d5b9a0bc1cf9d627 100644 --- a/core/modules/block_content/tests/src/Functional/BlockContentRevisionDeleteTest.php +++ b/core/modules/block_content/tests/src/Functional/BlockContentRevisionDeleteTest.php @@ -50,20 +50,31 @@ public function testDeleteForm(): void { $this->drupalGet($entity->toUrl('revision-delete-form')); $this->assertSession()->statusCodeEquals(403); - // Create a new latest revision. + // Create a new non default revision. $entity ->setRevisionCreationTime((new \DateTimeImmutable('11 January 2009 5pm'))->getTimestamp()) ->setRevisionTranslationAffected(TRUE) ->setNewRevision(); + $entity->isDefaultRevision(FALSE); $entity->save(); + $nonDefaultRevisionId = $entity->getRevisionId(); - // Reload the entity. + // Reload the default entity. $revision = \Drupal::entityTypeManager()->getStorage('block_content') ->loadRevision($revisionId); + // Cannot delete default revision. $this->drupalGet($revision->toUrl('revision-delete-form')); - $this->assertSession()->pageTextContains('Are you sure you want to delete the revision from Sun, 01/11/2009 - 16:00?'); + $this->assertSession()->statusCodeEquals(403); + $this->assertFalse($revision->access('delete revision', $this->adminUser, FALSE)); + + // Reload the non default entity. + $revision2 = \Drupal::entityTypeManager()->getStorage('block_content') + ->loadRevision($nonDefaultRevisionId); + $this->drupalGet($revision2->toUrl('revision-delete-form')); + $this->assertSession()->pageTextContains('Are you sure you want to delete the revision from Sun, 01/11/2009 - 17:00?'); $this->assertSession()->buttonExists('Delete'); $this->assertSession()->linkExists('Cancel'); + $this->assertTrue($revision2->access('delete revision', $this->adminUser, FALSE)); $countRevisions = static function (): int { return (int) \Drupal::entityTypeManager()->getStorage('block_content') @@ -79,7 +90,7 @@ public function testDeleteForm(): void { $this->assertEquals($count - 1, $countRevisions()); $this->assertSession()->statusCodeEquals(200); $this->assertSession()->addressEquals(sprintf('admin/content/block/%s/revisions', $entity->id())); - $this->assertSession()->pageTextContains(sprintf('Revision from Sun, 01/11/2009 - 16:00 of basic %s has been deleted.', $entity->label())); + $this->assertSession()->pageTextContains(sprintf('Revision from Sun, 01/11/2009 - 17:00 of basic %s has been deleted.', $entity->label())); $this->assertSession()->elementsCount('css', 'table tbody tr', 1); } diff --git a/core/modules/block_content/tests/src/Functional/BlockContentRevisionRevertTest.php b/core/modules/block_content/tests/src/Functional/BlockContentRevisionRevertTest.php index a6e77b0257a75c4b0eb007dbde56549d640e9ac5..6265cf967a2ea8f13957e55cc6db4d478373053a 100644 --- a/core/modules/block_content/tests/src/Functional/BlockContentRevisionRevertTest.php +++ b/core/modules/block_content/tests/src/Functional/BlockContentRevisionRevertTest.php @@ -50,20 +50,31 @@ public function testRevertForm(): void { $this->drupalGet($entity->toUrl('revision-revert-form')); $this->assertSession()->statusCodeEquals(403); - // Create a new latest revision. + // Create a new non default revision. $entity ->setRevisionCreationTime((new \DateTimeImmutable('11 January 2009 5pm'))->getTimestamp()) ->setRevisionTranslationAffected(TRUE) ->setNewRevision(); + $entity->isDefaultRevision(FALSE); $entity->save(); + $nonDefaultRevisionId = $entity->getRevisionId(); - // Reload the entity. + // Reload the default entity. $revision = \Drupal::entityTypeManager()->getStorage('block_content') ->loadRevision($revisionId); + // Cannot revert default revision. $this->drupalGet($revision->toUrl('revision-revert-form')); - $this->assertSession()->pageTextContains('Are you sure you want to revert to the revision from Sun, 01/11/2009 - 16:00?'); + $this->assertSession()->statusCodeEquals(403); + $this->assertFalse($revision->access('revert', $this->adminUser, FALSE)); + + // Reload the non default entity. + $revision2 = \Drupal::entityTypeManager()->getStorage('block_content') + ->loadRevision($nonDefaultRevisionId); + $this->drupalGet($revision2->toUrl('revision-revert-form')); + $this->assertSession()->pageTextContains('Are you sure you want to revert to the revision from Sun, 01/11/2009 - 17:00?'); $this->assertSession()->buttonExists('Revert'); $this->assertSession()->linkExists('Cancel'); + $this->assertTrue($revision2->access('revert', $this->adminUser, FALSE)); $countRevisions = static function (): int { return (int) \Drupal::entityTypeManager()->getStorage('block_content') @@ -79,7 +90,7 @@ public function testRevertForm(): void { $this->assertEquals($count + 1, $countRevisions()); $this->assertSession()->statusCodeEquals(200); $this->assertSession()->addressEquals(sprintf('admin/content/block/%s/revisions', $entity->id())); - $this->assertSession()->pageTextContains(sprintf('basic %s has been reverted to the revision from Sun, 01/11/2009 - 16:00.', $entity->label())); + $this->assertSession()->pageTextContains(sprintf('basic %s has been reverted to the revision from Sun, 01/11/2009 - 17:00.', $entity->label())); // Three rows, from the top: the newly reverted revision, the revision from // 5pm, and the revision from 4pm. $this->assertSession()->elementsCount('css', 'table tbody tr', 3); diff --git a/core/modules/system/tests/modules/entity_test/src/EntityTestAccessControlHandler.php b/core/modules/system/tests/modules/entity_test/src/EntityTestAccessControlHandler.php index ab4b7caa0fa83fb06c110b6e2ffb40e8ddf3ed19..96d6fa4f5bcf509b23455f267cc8e23d602f661d 100644 --- a/core/modules/system/tests/modules/entity_test/src/EntityTestAccessControlHandler.php +++ b/core/modules/system/tests/modules/entity_test/src/EntityTestAccessControlHandler.php @@ -79,12 +79,10 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter return AccessResult::allowedIf(in_array($operation, $labels, TRUE)); } elseif ($operation === 'revert') { - // Disallow reverting to latest. - return AccessResult::allowedIf(!$entity->isDefaultRevision() && !$entity->isLatestRevision() && in_array('revert', $labels, TRUE)); + return AccessResult::allowedIf(in_array('revert', $labels, TRUE)); } elseif ($operation === 'delete revision') { - // Disallow deleting latest and current revision. - return AccessResult::allowedIf(!$entity->isLatestRevision() && in_array('delete revision', $labels, TRUE)); + return AccessResult::allowedIf(in_array('delete revision', $labels, TRUE)); } // No opinion. diff --git a/core/modules/system/tests/modules/entity_test_revlog/src/EntityTestRevlogAccessControlHandler.php b/core/modules/system/tests/modules/entity_test_revlog/src/EntityTestRevlogAccessControlHandler.php index a5275600bc6170d877208780d68381f1ad6d42de..d0515d6afffbabb35d76c8dae0b08efa67c0b349 100644 --- a/core/modules/system/tests/modules/entity_test_revlog/src/EntityTestRevlogAccessControlHandler.php +++ b/core/modules/system/tests/modules/entity_test_revlog/src/EntityTestRevlogAccessControlHandler.php @@ -33,16 +33,12 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter } elseif ($operation === 'revert') { return AccessResult::allowedIf( - // Allow revert even if latest. - in_array('force allow revert', $labels, TRUE) || // Disallow reverting to latest. (!$entity->isDefaultRevision() && !$entity->isLatestRevision() && in_array('revert', $labels, TRUE)) ); } elseif ($operation === 'delete revision') { return AccessResult::allowedIf( - // Allow revision deletion even if latest. - in_array('force allow delete revision', $labels, TRUE) || // Disallow deleting latest and current revision. (!$entity->isLatestRevision() && in_array('delete revision', $labels, TRUE)) ); diff --git a/core/tests/Drupal/FunctionalTests/Entity/RevisionDeleteFormTest.php b/core/tests/Drupal/FunctionalTests/Entity/RevisionDeleteFormTest.php index 9c0620fb367a0901c2c77716a5ece2018652dd95..b956658c14be90eca43559a42be10ee0a6154324 100644 --- a/core/tests/Drupal/FunctionalTests/Entity/RevisionDeleteFormTest.php +++ b/core/tests/Drupal/FunctionalTests/Entity/RevisionDeleteFormTest.php @@ -95,7 +95,7 @@ public function providerPageTitle(): array { * * @covers \Drupal\Core\Entity\EntityAccessControlHandler::checkAccess */ - public function testAccessDeleteLatest(): void { + public function testAccessDeleteLatestDefault(): void { /** @var \Drupal\entity_test\Entity\EntityTestRev $entity */ $entity = EntityTestRev::create(); $entity->setName('delete revision'); @@ -108,6 +108,32 @@ public function testAccessDeleteLatest(): void { $this->assertSession()->statusCodeEquals(403); } + /** + * Ensure that forward revision can be deleted. + * + * @covers \Drupal\Core\Entity\EntityAccessControlHandler::checkAccess + */ + public function testAccessDeleteLatestForwardRevision(): void { + /** @var \Drupal\entity_test\Entity\EntityTestRevPub $entity */ + $entity = EntityTestRevPub::create(); + $entity->setName('delete revision'); + $entity->save(); + + $entity->isDefaultRevision(TRUE); + $entity->setPublished(); + $entity->setNewRevision(); + $entity->save(); + + $entity->isDefaultRevision(FALSE); + $entity->setUnpublished(); + $entity->setNewRevision(); + $entity->save(); + + $this->drupalGet($entity->toUrl('revision-delete-form')); + $this->assertSession()->statusCodeEquals(200); + $this->assertTrue($entity->access('delete revision', $this->rootUser, FALSE)); + } + /** * Test cannot delete default revision. * @@ -137,8 +163,9 @@ public function testAccessDeleteDefault(): void { // Check default but not latest. $this->assertTrue($revision->isDefaultRevision()); $this->assertFalse($revision->isLatestRevision()); - $this->drupalGet($entity->toUrl('revision-delete-form')); + $this->drupalGet($revision->toUrl('revision-delete-form')); $this->assertSession()->statusCodeEquals(403); + $this->assertFalse($revision->access('delete revision', $this->rootUser, FALSE)); } /** @@ -162,6 +189,7 @@ public function testAccessDeleteNonLatest(): void { ->loadRevision($revisionId); $this->drupalGet($revision->toUrl('revision-delete-form')); $this->assertSession()->statusCodeEquals(200); + $this->assertTrue($revision->access('delete revision', $this->rootUser, FALSE)); } /** diff --git a/core/tests/Drupal/FunctionalTests/Entity/RevisionRevertFormTest.php b/core/tests/Drupal/FunctionalTests/Entity/RevisionRevertFormTest.php index 98037f9c3078297824b136141ad020993a9504db..2c67d3ab4a984c7684d38bf459ec34950411bbd2 100644 --- a/core/tests/Drupal/FunctionalTests/Entity/RevisionRevertFormTest.php +++ b/core/tests/Drupal/FunctionalTests/Entity/RevisionRevertFormTest.php @@ -5,6 +5,7 @@ use Drupal\Component\Render\FormattableMarkup; use Drupal\Core\Entity\RevisionLogInterface; use Drupal\entity_test\Entity\EntityTestRev; +use Drupal\entity_test\Entity\EntityTestRevPub; use Drupal\entity_test_revlog\Entity\EntityTestWithRevisionLog; use Drupal\Tests\BrowserTestBase; @@ -91,11 +92,11 @@ public function providerPageTitle(): array { } /** - * Test cannot revert latest revision. + * Test cannot revert latest default revision. * * @covers \Drupal\Core\Entity\EntityAccessControlHandler::checkAccess */ - public function testAccessRevertLatest(): void { + public function testAccessRevertLatestDefault(): void { /** @var \Drupal\entity_test\Entity\EntityTestRev $entity */ $entity = EntityTestRev::create(); $entity->setName('revert'); @@ -106,6 +107,31 @@ public function testAccessRevertLatest(): void { $this->drupalGet($entity->toUrl('revision-revert-form')); $this->assertSession()->statusCodeEquals(403); + $this->assertFalse($entity->access('revert', $this->rootUser, FALSE)); + } + + /** + * Ensures that forward revisions can be reverted. + * + * @covers \Drupal\Core\Entity\EntityAccessControlHandler::checkAccess + */ + public function testAccessRevertLatestForwardRevision(): void { + /** @var \Drupal\entity_test\Entity\EntityTestRev $entity */ + $entity = EntityTestRevPub::create(); + $entity->setName('revert'); + $entity->isDefaultRevision(TRUE); + $entity->setPublished(); + $entity->setNewRevision(); + $entity->save(); + + $entity->isDefaultRevision(FALSE); + $entity->setUnpublished(); + $entity->setNewRevision(); + $entity->save(); + + $this->drupalGet($entity->toUrl('revision-revert-form')); + $this->assertSession()->statusCodeEquals(200); + $this->assertTrue($entity->access('revert', $this->rootUser, FALSE)); } /** @@ -128,6 +154,7 @@ public function testAccessRevertNonLatest(): void { ->loadRevision($revisionId); $this->drupalGet($revision->toUrl('revision-revert-form')); $this->assertSession()->statusCodeEquals(200); + $this->assertTrue($revision->access('revert', $this->rootUser, FALSE)); } /** diff --git a/core/tests/Drupal/FunctionalTests/Entity/RevisionVersionHistoryTest.php b/core/tests/Drupal/FunctionalTests/Entity/RevisionVersionHistoryTest.php index bcfb7e810f3ecfb1f32dac911148eb682291b372..f05a40abd0adeafbc0f5b817976540bcdc216332 100644 --- a/core/tests/Drupal/FunctionalTests/Entity/RevisionVersionHistoryTest.php +++ b/core/tests/Drupal/FunctionalTests/Entity/RevisionVersionHistoryTest.php @@ -261,17 +261,8 @@ public function testOperationRevertRevision(): void { $row3 = $this->assertSession()->elementExists('css', 'table tbody tr:nth-child(3)'); $this->assertSession()->elementNotExists('named', ['link', 'Revert'], $row3); - // Reverting latest is allowed if entity access permits it. - $entity->setName('view all revisions, revert, force allow revert'); - $entity->setNewRevision(); - $entity->save(); - $this->drupalGet($entity->toUrl('version-history')); - $this->assertSession()->elementsCount('css', 'table tbody tr', 4); - - $row1 = $this->assertSession()->elementExists('css', 'table tbody tr:nth-child(1)'); - $this->assertSession()->elementTextContains('css', 'table tbody tr:nth-child(1)', 'Current revision'); - $this->assertSession()->elementExists('named', ['link', 'Revert'], $row1); + $this->assertSession()->elementsCount('css', 'table tbody tr', 3); } /** @@ -309,18 +300,8 @@ public function testOperationDeleteRevision(): void { // Revision 3 does not have delete revision operation: no access. $row3 = $this->assertSession()->elementExists('css', 'table tbody tr:nth-child(3)'); $this->assertSession()->elementNotExists('named', ['link', 'Delete'], $row3); - - // Deleting latest is allowed if entity access permits it. - $entity->setName('view all revisions, delete revision, force allow delete revision'); - $entity->setNewRevision(); - $entity->save(); - $this->drupalGet($entity->toUrl('version-history')); - $this->assertSession()->elementsCount('css', 'table tbody tr', 4); - - $row1 = $this->assertSession()->elementExists('css', 'table tbody tr:nth-child(1)'); - $this->assertSession()->elementTextContains('css', 'table tbody tr:nth-child(1)', 'Current revision'); - $this->assertSession()->elementExists('named', ['link', 'Delete'], $row1); + $this->assertSession()->elementsCount('css', 'table tbody tr', 3); } }