From 6fb71f02089574cc4908845e2a865d59b21a8a5b Mon Sep 17 00:00:00 2001 From: catch <catch@35733.no-reply.drupal.org> Date: Tue, 7 Sep 2021 14:42:18 +0100 Subject: [PATCH] Issue #3226487 by acbramley, bbrala, Berdir, AaronMcHale, darvanen, catch, daffie, mstrelan: Always allow access to revisions based on access/permissions, not on the count of revisions --- .../src/Access/MediaRevisionAccessCheck.php | 9 +---- .../src/Functional/MediaRevisionTest.php | 4 +- .../src/Access/NodeRevisionAccessCheck.php | 33 ++++++---------- .../src/Functional/NodeRevisionsTest.php | 5 +++ .../NodeRevisionsUiBypassAccessTest.php | 38 +------------------ 5 files changed, 20 insertions(+), 69 deletions(-) diff --git a/core/modules/media/src/Access/MediaRevisionAccessCheck.php b/core/modules/media/src/Access/MediaRevisionAccessCheck.php index f3daad20f413..b4d4daad3f6b 100644 --- a/core/modules/media/src/Access/MediaRevisionAccessCheck.php +++ b/core/modules/media/src/Access/MediaRevisionAccessCheck.php @@ -108,14 +108,7 @@ public function checkAccess(MediaInterface $media, AccountInterface $account, $o return FALSE; } - // There should be at least two revisions. If the revision ID of the - // given media item and the revision ID of the default revision differ, - // then we already have two different revisions so there is no need for a - // separate database check. - if ($media->isDefaultRevision() && ($this->countDefaultLanguageRevisions($media) == 1)) { - $this->access[$cid] = FALSE; - } - elseif ($account->hasPermission('administer media')) { + if ($account->hasPermission('administer media')) { $this->access[$cid] = TRUE; } else { diff --git a/core/modules/media/tests/src/Functional/MediaRevisionTest.php b/core/modules/media/tests/src/Functional/MediaRevisionTest.php index c08b392a63bf..bf7028a90eb2 100644 --- a/core/modules/media/tests/src/Functional/MediaRevisionTest.php +++ b/core/modules/media/tests/src/Functional/MediaRevisionTest.php @@ -37,9 +37,9 @@ public function testRevisions() { ]); $media->save(); - // You can't access the revision page when there is only 1 revision. + // You can access the revision page when there is only 1 revision. $this->drupalGet('media/' . $media->id() . '/revisions/' . $media->getRevisionId() . '/view'); - $assert->statusCodeEquals(403); + $assert->statusCodeEquals(200); // Create some revisions. $media_revisions = []; diff --git a/core/modules/node/src/Access/NodeRevisionAccessCheck.php b/core/modules/node/src/Access/NodeRevisionAccessCheck.php index f70b9e359ce4..31a2f2941dc9 100644 --- a/core/modules/node/src/Access/NodeRevisionAccessCheck.php +++ b/core/modules/node/src/Access/NodeRevisionAccessCheck.php @@ -119,31 +119,20 @@ public function checkAccess(NodeInterface $node, AccountInterface $account, $op $this->access[$cid] = FALSE; return FALSE; } - // If the revisions checkbox is selected for the content type, display the - // revisions tab. - $bundle_entity_type = $node->getEntityType()->getBundleEntityType(); - $bundle_entity = \Drupal::entityTypeManager()->getStorage($bundle_entity_type)->load($bundle); - if ($bundle_entity->shouldCreateNewRevision() && $op === 'view') { + + // If this is the default revision, return access denied for revert or + // delete operations. + if ($node->isDefaultRevision() && ($op === 'update' || $op === 'delete')) { + $this->access[$cid] = FALSE; + } + elseif ($account->hasPermission('administer nodes')) { $this->access[$cid] = TRUE; } else { - // There should be at least two revisions. If the vid of the given node - // and the vid of the default revision differ, then we already have two - // different revisions so there is no need for a separate database - // check. Also, if you try to revert to or delete the default revision, - // that's not good. - if ($node->isDefaultRevision() && ($this->nodeStorage->countDefaultLanguageRevisions($node) == 1 || $op === 'update' || $op === 'delete')) { - $this->access[$cid] = FALSE; - } - elseif ($account->hasPermission('administer nodes')) { - $this->access[$cid] = TRUE; - } - else { - // First check the access to the default revision and finally, if the - // node passed in is not the default revision then check access to - // that, too. - $this->access[$cid] = $this->nodeAccess->access($this->nodeStorage->load($node->id()), $op, $account) && ($node->isDefaultRevision() || $this->nodeAccess->access($node, $op, $account)); - } + // First check the access to the default revision and finally, if the + // node passed in is not the default revision then check access to + // that, too. + $this->access[$cid] = $this->nodeAccess->access($this->nodeStorage->load($node->id()), $op, $account) && ($node->isDefaultRevision() || $this->nodeAccess->access($node, $op, $account)); } } diff --git a/core/modules/node/tests/src/Functional/NodeRevisionsTest.php b/core/modules/node/tests/src/Functional/NodeRevisionsTest.php index fb0929bc09a4..e3bcc483002b 100644 --- a/core/modules/node/tests/src/Functional/NodeRevisionsTest.php +++ b/core/modules/node/tests/src/Functional/NodeRevisionsTest.php @@ -145,6 +145,11 @@ protected function setUp(): void { * Checks node revision related operations. */ public function testRevisions() { + // Access to the revision page for a node with 1 revision is allowed. + $node = $this->drupalCreateNode(); + $this->drupalGet("node/" . $node->id() . "/revisions/" . $node->getRevisionId() . "/view"); + $this->assertSession()->statusCodeEquals(200); + $node_storage = $this->container->get('entity_type.manager')->getStorage('node'); $nodes = $this->nodes; $logs = $this->revisionLogs; diff --git a/core/modules/node/tests/src/Functional/NodeRevisionsUiBypassAccessTest.php b/core/modules/node/tests/src/Functional/NodeRevisionsUiBypassAccessTest.php index 36698b3073bc..3bb0f08f63eb 100644 --- a/core/modules/node/tests/src/Functional/NodeRevisionsUiBypassAccessTest.php +++ b/core/modules/node/tests/src/Functional/NodeRevisionsUiBypassAccessTest.php @@ -75,7 +75,7 @@ public function testDisplayRevisionTab() { $this->submitForm($edit, 'Save'); $this->assertSession()->addressEquals($node->toUrl()); - // Verify revisions exist since the content type has revisions enabled. + // Verify revisions exist. $this->assertSession()->linkExists('Revisions'); // Verify the checkbox is checked on the node edit form. @@ -89,42 +89,6 @@ public function testDisplayRevisionTab() { $this->assertSession()->addressEquals($node->toUrl()); $this->assertSession()->linkExists('Revisions'); - - // Unset page revision setting 'create new revision'. This will mean new - // revisions are not created by default when the node is edited. - $type = NodeType::load('page'); - $type->setNewRevision(FALSE); - $type->save(); - - // Create the node. - $node = $this->drupalCreateNode(); - - // Verify the checkbox is unchecked on the node edit form. - $this->drupalGet('node/' . $node->id() . '/edit'); - $this->assertSession()->checkboxNotChecked('edit-revision'); - // Submit the form without changing the checkbox. - $edit = []; - $this->drupalGet('node/' . $node->id() . '/edit'); - $this->submitForm($edit, 'Save'); - - $this->assertSession()->addressEquals($node->toUrl()); - // Verify that no link to revisions is displayed since the type - // has the 'create new revision' setting unset. - $this->assertSession()->linkNotExists('Revisions'); - - // Verify the checkbox is unchecked on the node edit form. - $this->drupalGet('node/' . $node->id() . '/edit'); - $this->assertSession()->checkboxNotChecked('edit-revision'); - - // Check the 'create new revision' checkbox and save the node. - $edit = ['revision' => TRUE]; - $this->drupalGet('node/' . $node->id() . '/edit'); - $this->submitForm($edit, 'Save'); - - $this->assertSession()->addressEquals($node->toUrl()); - // Verify that the link is displayed since a new revision is created and - // the 'create new revision' checkbox on the node is checked. - $this->assertSession()->linkExists('Revisions'); } } -- GitLab