SA-CORE-2021-009 by illeace, Wim Leers, xjm, effulgentsia, larowlan, …

…pandaski, vijaycs85, phenaproxima, mcdruid
drupal · Sep 14, 2021 · 8e8e3d2 · 8e8e3d2
1 parent 634daa8
commit 8e8e3d2
Show file tree

Hide file tree

Showing 7 changed files with 106 additions and 51 deletions.
diff --git a/modules/quickedit/src/MetadataGenerator.php b/modules/quickedit/src/MetadataGenerator.php
@@ -68,7 +68,7 @@ public function generateFieldMetadata(FieldItemListInterface $items, $view_mode)
 
     // Early-return if user does not have access.
     $access = $this->accessChecker->accessEditEntityField($entity, $field_name);
-    if (!$access) {
+    if (!$access->isAllowed()) {
       return ['access' => FALSE];
     }
 

diff --git a/modules/quickedit/tests/modules/src/MockQuickEditEntityFieldAccessCheck.php b/modules/quickedit/tests/modules/src/MockQuickEditEntityFieldAccessCheck.php
@@ -2,6 +2,7 @@
 
 namespace Drupal\quickedit_test;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Entity\EntityInterface;
 use Drupal\quickedit\Access\QuickEditEntityFieldAccessCheckInterface;
 
@@ -14,7 +15,19 @@ class MockQuickEditEntityFieldAccessCheck implements QuickEditEntityFieldAccessC
    * {@inheritdoc}
    */
   public function accessEditEntityField(EntityInterface $entity, $field_name) {
-    return TRUE;
+    switch (\Drupal::state()->get('quickedit_test_field_access')) {
+      case 'allowed':
+        return AccessResult::allowed();
+
+      case 'neutral':
+        return AccessResult::neutral();
+
+      case 'forbidden':
+        return AccessResult::forbidden();
+
+      default:
+        throw new \OutOfRangeException("The state for the 'quickedit_test_field_access' key must be either 'allowed', 'neutral' or 'forbidden'.");
+    }
   }
 
 }
diff --git a/modules/quickedit/tests/src/FunctionalJavascript/LayoutBuilderQuickEditTest.php b/modules/quickedit/tests/src/FunctionalJavascript/LayoutBuilderQuickEditTest.php
@@ -112,6 +112,14 @@ public function testQuickEditIgnoresDuplicateFields() {
 
     $this->drupalLogin($this->contentAuthorUser);
     $this->usingLayoutBuilder = TRUE;
+    $this->assertQuickEditInit(['title']);
+    $this->drupalLogin($this->drupalCreateUser([
+      'access contextual links',
+      'access in-place editing',
+      'access content',
+      'edit any article content',
+      'administer nodes',
+    ]));
     $this->assertQuickEditInit(['title', 'uid', 'created']);
   }
 
@@ -123,18 +131,26 @@ public function testQuickEditIgnoresDuplicateFields() {
    *
    * @dataProvider providerEnableDisableLayoutBuilder
    */
-  public function testEnableDisableLayoutBuilder($use_revisions) {
+  public function testEnableDisableLayoutBuilder($use_revisions, $admin_permission = FALSE) {
     if (!$use_revisions) {
       $content_type = NodeType::load('article');
       $content_type->setNewRevision(FALSE);
       $content_type->save();
     }
     $fields = [
       'title',
-      'uid',
-      'created',
       'body',
     ];
+    if ($admin_permission) {
+      $fields = array_merge($fields, ['uid', 'created']);
+      $this->drupalLogin($this->drupalCreateUser([
+        'access contextual links',
+        'access in-place editing',
+        'access content',
+        'edit any article content',
+        'administer nodes',
+      ]));
+    }
 
     // Test article with Layout Builder disabled.
     $this->assertQuickEditInit($fields);
@@ -168,8 +184,10 @@ public function testEnableDisableLayoutBuilder($use_revisions) {
    */
   public function providerEnableDisableLayoutBuilder() {
     return [
-      'use revisions' => [TRUE],
-      'do not use revisions' => [FALSE],
+      'use revisions, not admin' => [TRUE],
+      'do not use revisions, not admin' => [FALSE],
+      'use revisions, admin' => [TRUE, TRUE],
+      'do not use revisions, admin' => [FALSE, TRUE],
     ];
   }
 

diff --git a/modules/quickedit/tests/src/FunctionalJavascript/QuickEditImageTest.php b/modules/quickedit/tests/src/FunctionalJavascript/QuickEditImageTest.php
@@ -41,27 +41,32 @@ protected function setUp(): void {
 
     // Create the Article node type.
     $this->drupalCreateContentType(['type' => 'article', 'name' => 'Article']);
+  }
 
+  /**
+   * Tests that quick editor works correctly with images.
+   *
+   * @covers ::isCompatible
+   * @covers ::getAttachments
+   *
+   * @dataProvider providerTestImageInPlaceEditor
+   */
+  public function testImageInPlaceEditor($admin_permission = FALSE) {
     // Log in as a content author who can use Quick Edit and edit Articles.
-    $this->contentAuthorUser = $this->drupalCreateUser([
+    $permissions = [
       'access contextual links',
       'access toolbar',
       'access in-place editing',
       'access content',
       'create article content',
       'edit any article content',
       'delete any article content',
-    ]);
+    ];
+    if ($admin_permission) {
+      $permissions[] = 'administer nodes';
+    }
+    $this->contentAuthorUser = $this->drupalCreateUser($permissions);
     $this->drupalLogin($this->contentAuthorUser);
-  }
-
-  /**
-   * Tests that quick editor works correctly with images.
-   *
-   * @covers ::isCompatible
-   * @covers ::getAttachments
-   */
-  public function testImageInPlaceEditor() {
     // Create a field with a basic filetype restriction.
     $field_name = strtolower($this->randomMachineName());
     $field_settings = [
@@ -126,13 +131,25 @@ public function testImageInPlaceEditor() {
     $this->assertEntityInstanceStates([
       'node/1[0]' => 'closed',
     ]);
+
+    $admin_inactive = [];
+    $admin_candidate = [];
+    if ($admin_permission) {
+      $admin_inactive = [
+        'node/1/uid/en/full' => 'inactive',
+        'node/1/created/en/full' => 'inactive',
+      ];
+      $admin_candidate = [
+        'node/1/uid/en/full' => 'candidate',
+        'node/1/created/en/full' => 'candidate',
+      ];
+    }
+
     $this->assertEntityInstanceFieldStates('node', 1, 0, [
       'node/1/title/en/full'               => 'inactive',
-      'node/1/uid/en/full'                 => 'inactive',
-      'node/1/created/en/full'             => 'inactive',
       'node/1/body/en/full'                => 'inactive',
       'node/1/' . $field_name . '/en/full' => 'inactive',
-    ]);
+    ] + $admin_inactive);
 
     // Start in-place editing of the article node.
     $this->startQuickEditViaToolbar('node', 1, 0);
@@ -142,33 +159,27 @@ public function testImageInPlaceEditor() {
     $this->assertQuickEditEntityToolbar((string) $node->label(), NULL);
     $this->assertEntityInstanceFieldStates('node', 1, 0, [
       'node/1/title/en/full'               => 'candidate',
-      'node/1/uid/en/full'                 => 'candidate',
-      'node/1/created/en/full'             => 'candidate',
       'node/1/body/en/full'                => 'candidate',
       'node/1/' . $field_name . '/en/full' => 'candidate',
-    ]);
+    ] + $admin_candidate);
 
     // Click the image field.
     $this->click($field_selector);
     $this->awaitImageEditor();
     $this->assertSession()->elementExists('css', $field_selector . ' .quickedit-image-dropzone');
     $this->assertEntityInstanceFieldStates('node', 1, 0, [
       'node/1/title/en/full'               => 'candidate',
-      'node/1/uid/en/full'                 => 'candidate',
-      'node/1/created/en/full'             => 'candidate',
       'node/1/body/en/full'                => 'candidate',
       'node/1/' . $field_name . '/en/full' => 'active',
-    ]);
+    ] + $admin_candidate);
 
     // Type new 'alt' text.
     $this->typeInImageEditorAltTextInput('New text');
     $this->assertEntityInstanceFieldStates('node', 1, 0, [
       'node/1/title/en/full'               => 'candidate',
-      'node/1/uid/en/full'                 => 'candidate',
-      'node/1/created/en/full'             => 'candidate',
       'node/1/body/en/full'                => 'candidate',
       'node/1/' . $field_name . '/en/full' => 'changed',
-    ]);
+    ] + $admin_candidate);
 
     // Drag and drop an image.
     $this->dropImageOnImageEditor($valid_images[1]->uri);
@@ -184,11 +195,9 @@ public function testImageInPlaceEditor() {
     ]);
     $this->assertEntityInstanceFieldStates('node', 1, 0, [
       'node/1/title/en/full'               => 'candidate',
-      'node/1/uid/en/full'                 => 'candidate',
-      'node/1/created/en/full'             => 'candidate',
       'node/1/body/en/full'                => 'candidate',
       'node/1/' . $field_name . '/en/full' => 'saving',
-    ]);
+    ] + $admin_candidate);
     $this->assertEntityInstanceFieldMarkup([
       'node/1/' . $field_name . '/en/full' => '.quickedit-changed',
     ]);
@@ -208,4 +217,17 @@ public function testImageInPlaceEditor() {
     $this->assertSession()->elementExists('css', $entity_selector . ' ' . $field_selector . ' ' . $new_image_selector);
   }
 
+  /**
+   * Data provider for ::testImageInPlaceEditor().
+   *
+   * @return array
+   *   Test cases.
+   */
+  public function providerTestImageInPlaceEditor(): array {
+    return [
+      'with permission' => [TRUE],
+      'without permission' => [FALSE],
+    ];
+  }
+
 }
diff --git a/modules/quickedit/tests/src/FunctionalJavascript/QuickEditIntegrationTest.php b/modules/quickedit/tests/src/FunctionalJavascript/QuickEditIntegrationTest.php
@@ -146,8 +146,6 @@ public function testArticleNode() {
     ]);
     $this->assertEntityInstanceFieldStates('node', 1, 0, [
       'node/1/title/en/full'      => 'inactive',
-      'node/1/uid/en/full'        => 'inactive',
-      'node/1/created/en/full'    => 'inactive',
       'node/1/body/en/full'       => 'inactive',
       'node/1/field_tags/en/full' => 'inactive',
     ]);
@@ -160,8 +158,6 @@ public function testArticleNode() {
     $this->assertQuickEditEntityToolbar((string) $node->label(), NULL);
     $this->assertEntityInstanceFieldStates('node', 1, 0, [
       'node/1/title/en/full'      => 'candidate',
-      'node/1/uid/en/full'        => 'candidate',
-      'node/1/created/en/full'    => 'candidate',
       'node/1/body/en/full'       => 'candidate',
       'node/1/field_tags/en/full' => 'candidate',
     ]);
@@ -174,8 +170,6 @@ public function testArticleNode() {
     $this->assertQuickEditEntityToolbar((string) $node->label(), 'Title');
     $this->assertEntityInstanceFieldStates('node', 1, 0, [
       'node/1/title/en/full'      => 'active',
-      'node/1/uid/en/full'        => 'candidate',
-      'node/1/created/en/full'    => 'candidate',
       'node/1/body/en/full'       => 'candidate',
       'node/1/field_tags/en/full' => 'candidate',
     ]);
@@ -188,8 +182,6 @@ public function testArticleNode() {
     $this->awaitEntityInstanceFieldState('node', 1, 0, 'title', 'en', 'changed');
     $this->assertEntityInstanceFieldStates('node', 1, 0, [
       'node/1/title/en/full'      => 'changed',
-      'node/1/uid/en/full'        => 'candidate',
-      'node/1/created/en/full'    => 'candidate',
       'node/1/body/en/full'       => 'candidate',
       'node/1/field_tags/en/full' => 'candidate',
     ]);
@@ -201,8 +193,6 @@ public function testArticleNode() {
     $this->assertQuickEditEntityToolbar((string) $node->label(), 'Body');
     $this->assertEntityInstanceFieldStates('node', 1, 0, [
       'node/1/title/en/full'      => 'saving',
-      'node/1/uid/en/full'        => 'candidate',
-      'node/1/created/en/full'    => 'candidate',
       'node/1/body/en/full'       => 'active',
       'node/1/field_tags/en/full' => 'candidate',
     ]);
@@ -223,8 +213,6 @@ public function testArticleNode() {
     $assert_session->waitForElement('css', '.quickedit-toolbar-field div[id*="tags"]');
     $this->assertQuickEditEntityToolbar((string) $node->label(), 'Tags');
     $this->assertEntityInstanceFieldStates('node', 1, 0, [
-      'node/1/uid/en/full'        => 'candidate',
-      'node/1/created/en/full'    => 'candidate',
       'node/1/body/en/full'       => 'candidate',
       'node/1/field_tags/en/full' => 'activating',
       'node/1/title/en/full'      => 'candidate',
@@ -239,8 +227,6 @@ public function testArticleNode() {
     // Wait for the form to load.
     $this->assertJsCondition('document.querySelector(\'.quickedit-form-container > .quickedit-form[role="dialog"] > .placeholder\') === null');
     $this->assertEntityInstanceFieldStates('node', 1, 0, [
-      'node/1/uid/en/full'        => 'candidate',
-      'node/1/created/en/full'    => 'candidate',
       'node/1/body/en/full'       => 'candidate',
       'node/1/field_tags/en/full' => 'active',
       'node/1/title/en/full'      => 'candidate',
@@ -250,8 +236,6 @@ public function testArticleNode() {
     $this->typeInFormEditorTextInputField('field_tags[target_id]', 'foo, bar');
     $this->awaitEntityInstanceFieldState('node', 1, 0, 'field_tags', 'en', 'changed');
     $this->assertEntityInstanceFieldStates('node', 1, 0, [
-      'node/1/uid/en/full'        => 'candidate',
-      'node/1/created/en/full'    => 'candidate',
       'node/1/body/en/full'       => 'candidate',
       'node/1/field_tags/en/full' => 'changed',
       'node/1/title/en/full'      => 'candidate',
@@ -264,8 +248,6 @@ public function testArticleNode() {
       'node/1[0]' => 'committing',
     ]);
     $this->assertEntityInstanceFieldStates('node', 1, 0, [
-      'node/1/uid/en/full'        => 'candidate',
-      'node/1/created/en/full'    => 'candidate',
       'node/1/body/en/full'       => 'candidate',
       'node/1/field_tags/en/full' => 'saving',
       'node/1/title/en/full'      => 'candidate',

diff --git a/modules/quickedit/tests/src/Kernel/EditorIntegrationTest.php b/modules/quickedit/tests/src/Kernel/EditorIntegrationTest.php
@@ -179,6 +179,11 @@ public function testMetadata() {
 
     // Verify metadata.
     $items = $entity->get($this->fieldName);
+    \Drupal::state()->set('quickedit_test_field_access', 'forbidden');
+    $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items, 'default'));
+    \Drupal::state()->set('quickedit_test_field_access', 'neutral');
+    $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items, 'default'));
+    \Drupal::state()->set('quickedit_test_field_access', 'allowed');
     $metadata = $this->metadataGenerator->generateFieldMetadata($items, 'default');
     $expected = [
       'access' => TRUE,

diff --git a/modules/quickedit/tests/src/Kernel/MetadataGeneratorTest.php b/modules/quickedit/tests/src/Kernel/MetadataGeneratorTest.php
@@ -97,6 +97,11 @@ public function testSimpleEntityType() {
 
     // Verify metadata for field 1.
     $items_1 = $entity->get($field_1_name);
+    \Drupal::state()->set('quickedit_test_field_access', 'forbidden');
+    $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items_1, 'default'));
+    \Drupal::state()->set('quickedit_test_field_access', 'neutral');
+    $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items_1, 'default'));
+    \Drupal::state()->set('quickedit_test_field_access', 'allowed');
     $metadata_1 = $this->metadataGenerator->generateFieldMetadata($items_1, 'default');
     $expected_1 = [
       'access' => TRUE,
@@ -107,6 +112,11 @@ public function testSimpleEntityType() {
 
     // Verify metadata for field 2.
     $items_2 = $entity->get($field_2_name);
+    \Drupal::state()->set('quickedit_test_field_access', 'forbidden');
+    $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items_2, 'default'));
+    \Drupal::state()->set('quickedit_test_field_access', 'neutral');
+    $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items_2, 'default'));
+    \Drupal::state()->set('quickedit_test_field_access', 'allowed');
     $metadata_2 = $this->metadataGenerator->generateFieldMetadata($items_2, 'default');
     $expected_2 = [
       'access' => TRUE,
@@ -163,6 +173,11 @@ public function testEditorWithCustomMetadata() {
 
     // Verify metadata.
     $items = $entity->get($field_name);
+    \Drupal::state()->set('quickedit_test_field_access', 'forbidden');
+    $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items, 'default'));
+    \Drupal::state()->set('quickedit_test_field_access', 'neutral');
+    $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items, 'default'));
+    \Drupal::state()->set('quickedit_test_field_access', 'allowed');
     $metadata = $this->metadataGenerator->generateFieldMetadata($items, 'default');
     $expected = [
       'access' => TRUE,